diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9d8d57b6a..c21128086 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -160,6 +160,21 @@ func (in *PostgresUser) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresUserAWSSpec) DeepCopyInto(out *PostgresUserAWSSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresUserAWSSpec. +func (in *PostgresUserAWSSpec) DeepCopy() *PostgresUserAWSSpec { + if in == nil { + return nil + } + out := new(PostgresUserAWSSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresUserList) DeepCopyInto(out *PostgresUserList) { *out = *in @@ -202,6 +217,11 @@ func (in *PostgresUserSpec) DeepCopyInto(out *PostgresUserSpec) { (*out)[key] = val } } + if in.AWS != nil { + in, out := &in.AWS, &out.AWS + *out = new(PostgresUserAWSSpec) + **out = **in + } if in.Annotations != nil { in, out := &in.Annotations, &out.Annotations *out = make(map[string]string, len(*in)) diff --git a/cmd/main.go b/cmd/main.go index c445a64a2..229000e75 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -15,6 +15,7 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" + logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" @@ -28,10 +29,7 @@ import ( // +kubebuilder:scaffold:imports ) -var ( - scheme = runtime.NewScheme() - setupLog = ctrl.Log.WithName("setup") -) +var scheme = runtime.NewScheme() func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) @@ -63,8 +61,8 @@ func main() { opts.BindFlags(flag.CommandLine) flag.Parse() - ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) - + logger := zap.New(zap.UseFlagOptions(&opts)) + logf.SetLogger(logger) // if the enable-http2 flag is false (the default), http/2 should be disabled // due to its vulnerabilities. More specifically, disabling http/2 will // prevent from being vulnerable to the HTTP/2 Stream Cancellation and @@ -72,7 +70,7 @@ func main() { // - https://github.com/advisories/GHSA-qppj-fm5r-hxr3 // - https://github.com/advisories/GHSA-4374-p667-p6c8 disableHTTP2 := func(c *tls.Config) { - setupLog.Info("disabling http/2") + logger.Info("disabling http/2") c.NextProtos = []string{"http/1.1"} } @@ -111,9 +109,9 @@ func main() { cfg := config.Get() lockName := "lock" if cfg.AnnotationFilter == "" { - setupLog.Info("No POSTGRES_INSTANCE set, this instance will only process CRs without an annotation") + logger.Info("No POSTGRES_INSTANCE set, this instance will only process CRs without an annotation") } else { - setupLog.Info("POSTGRES_INSTANCE is set, this instance will only process CRs with the correct annotation", "annotation", cfg.AnnotationFilter) + logger.Info("POSTGRES_INSTANCE is set, this instance will only process CRs with the correct annotation", "annotation", cfg.AnnotationFilter) lockName += "-" + cfg.AnnotationFilter } cacheOpts := cache.Options{} @@ -145,38 +143,45 @@ func main() { // LeaderElectionReleaseOnCancel: true, }) if err != nil { - setupLog.Error(err, "unable to start manager") + logger.Error(err, "unable to start manager") os.Exit(1) } - pg, err := postgres.NewPG(cfg, ctrl.Log) + pg, err := postgres.NewPG(cfg, logger) if err != nil { - setupLog.Error(err, "DB-Connection failed", "cfg", cfg) + // Avoid logging sensitive information like PostgresPass + logger.Error(err, "DB-Connection failed", "cfg", map[string]any{ + "Host": cfg.PostgresHost, + "User": cfg.PostgresUser, + "UriArgs": cfg.PostgresUriArgs, + "CloudPriver": cfg.CloudProvider, + "DefaultDatabase": cfg.PostgresDefaultDb, + }) os.Exit(1) } if err = (controller.NewPostgresReconciler(mgr, cfg, pg)).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "Postgres") + logger.Error(err, "unable to create controller", "controller", "Postgres") os.Exit(1) } if err = (controller.NewPostgresUserReconciler(mgr, cfg, pg)).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "PostgresUser") + logger.Error(err, "unable to create controller", "controller", "PostgresUser") os.Exit(1) } // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { - setupLog.Error(err, "unable to set up health check") + logger.Error(err, "unable to set up health check") os.Exit(1) } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { - setupLog.Error(err, "unable to set up ready check") + logger.Error(err, "unable to set up ready check") os.Exit(1) } - setupLog.Info("starting manager") + logger.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - setupLog.Error(err, "problem running manager") + logger.Error(err, "problem running manager") os.Exit(1) } } diff --git a/config/crd/bases/db.movetokube.com_postgres.yaml b/config/crd/bases/db.movetokube.com_postgres.yaml index 209ed20b7..10b1f2585 100644 --- a/config/crd/bases/db.movetokube.com_postgres.yaml +++ b/config/crd/bases/db.movetokube.com_postgres.yaml @@ -20,14 +20,19 @@ spec: description: Postgres is the Schema for the postgres API properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string metadata: type: object diff --git a/config/crd/bases/db.movetokube.com_postgresusers.yaml b/config/crd/bases/db.movetokube.com_postgresusers.yaml index 0cbd8510a..bb466d47e 100644 --- a/config/crd/bases/db.movetokube.com_postgresusers.yaml +++ b/config/crd/bases/db.movetokube.com_postgresusers.yaml @@ -20,14 +20,19 @@ spec: description: PostgresUser is the Schema for the postgresusers API properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string metadata: type: object @@ -39,28 +44,23 @@ spec: type: string type: object aws: - description: AWS specific settings for this user. + description: PostgresUserAWSSpec encapsulates AWS specific configuration + toggles. properties: enableIamAuth: - description: Enable IAM authentication for this user (PostgreSQL on AWS RDS only) - default: false type: boolean type: object database: - description: Name of the PostgresDatabase this user will be related to type: string labels: additionalProperties: type: string type: object privileges: - description: List of privileges to grant to this user type: string role: - description: Name of the PostgresRole this user will be associated with type: string secretName: - description: Name of the secret to create with user credentials type: string secretTemplate: additionalProperties: @@ -74,11 +74,10 @@ spec: status: description: PostgresUserStatus defines the observed state of PostgresUser properties: - enableIamAuth: - description: Reflects whether IAM authentication is enabled for this user. - type: boolean databaseName: type: string + enableIamAuth: + type: boolean postgresGroup: type: string postgresLogin: @@ -89,6 +88,7 @@ spec: type: boolean required: - databaseName + - enableIamAuth - postgresGroup - postgresLogin - postgresRole diff --git a/internal/controller/postgres_controller.go b/internal/controller/postgres_controller.go index 151aaea15..01532dcc9 100644 --- a/internal/controller/postgres_controller.go +++ b/internal/controller/postgres_controller.go @@ -82,27 +82,27 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if !instance.GetDeletionTimestamp().IsZero() { if r.shouldDropDB(ctx, instance, reqLogger) && instance.Status.Succeeded { if instance.Status.Roles.Owner != "" { - err := r.pg.DropRole(instance.Status.Roles.Owner, r.pg.GetUser(), instance.Spec.Database, reqLogger) + err := r.pg.DropRole(instance.Status.Roles.Owner, r.pg.GetUser(), instance.Spec.Database) if err != nil { return ctrl.Result{}, err } instance.Status.Roles.Owner = "" } if instance.Status.Roles.Reader != "" { - err = r.pg.DropRole(instance.Status.Roles.Reader, r.pg.GetUser(), instance.Spec.Database, reqLogger) + err = r.pg.DropRole(instance.Status.Roles.Reader, r.pg.GetUser(), instance.Spec.Database) if err != nil { return ctrl.Result{}, err } instance.Status.Roles.Reader = "" } if instance.Status.Roles.Writer != "" { - err = r.pg.DropRole(instance.Status.Roles.Writer, r.pg.GetUser(), instance.Spec.Database, reqLogger) + err = r.pg.DropRole(instance.Status.Roles.Writer, r.pg.GetUser(), instance.Spec.Database) if err != nil { return ctrl.Result{}, err } instance.Status.Roles.Writer = "" } - err = r.pg.DropDatabase(instance.Spec.Database, reqLogger) + err = r.pg.DropDatabase(instance.Spec.Database) if err != nil { return ctrl.Result{}, err } @@ -175,7 +175,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c continue } // Execute create extension SQL statement - err = r.pg.CreateExtension(instance.Spec.Database, extension, reqLogger) + err = r.pg.CreateExtension(instance.Spec.Database, extension) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not add extensions %s", extension)) continue @@ -198,7 +198,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } // Create schema - err = r.pg.CreateSchema(database, owner, schema, reqLogger) + err = r.pg.CreateSchema(database, owner, schema) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not create schema %s", schema)) continue @@ -212,7 +212,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c Privs: readerPrivs, CreateSchema: false, } - err = r.pg.SetSchemaPrivileges(schemaPrivilegesReader, reqLogger) + err = r.pg.SetSchemaPrivileges(schemaPrivilegesReader) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", reader, readerPrivs)) continue @@ -224,7 +224,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c Privs: writerPrivs, CreateSchema: true, } - err = r.pg.SetSchemaPrivileges(schemaPrivilegesWriter, reqLogger) + err = r.pg.SetSchemaPrivileges(schemaPrivilegesWriter) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) continue @@ -236,7 +236,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c Privs: writerPrivs, CreateSchema: true, } - err = r.pg.SetSchemaPrivileges(schemaPrivilegesOwner, reqLogger) + err = r.pg.SetSchemaPrivileges(schemaPrivilegesOwner) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) continue @@ -259,6 +259,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c reqLogger.Info("Reconciling done") return ctrl.Result{}, nil } + func (r *PostgresReconciler) addFinalizer(reqLogger logr.Logger, m *dbv1alpha1.Postgres) error { if len(m.GetFinalizers()) < 1 && m.GetDeletionTimestamp() == nil { reqLogger.Info("adding Finalizer for Postgres") @@ -266,6 +267,7 @@ func (r *PostgresReconciler) addFinalizer(reqLogger logr.Logger, m *dbv1alpha1.P } return nil } + func (r *PostgresReconciler) requeue(cr *dbv1alpha1.Postgres, reason error) (ctrl.Result, error) { cr.Status.Succeeded = false return ctrl.Result{}, reason diff --git a/internal/controller/postgres_controller_test.go b/internal/controller/postgres_controller_test.go index 86e2bfa3e..6fd744104 100644 --- a/internal/controller/postgres_controller_test.go +++ b/internal/controller/postgres_controller_test.go @@ -142,10 +142,7 @@ var _ = Describe("PostgresReconciler", func() { }) Describe("Checking deletion logic", func() { - - var ( - postgresCR *v1alpha1.Postgres - ) + var postgresCR *v1alpha1.Postgres BeforeEach(func() { postgresCR = &v1alpha1.Postgres{ @@ -165,11 +162,9 @@ var _ = Describe("PostgresReconciler", func() { }, }, } - }) Context("DropOnDelete is unset", func() { - BeforeEach(func() { initClient(postgresCR, true) }) @@ -194,17 +189,15 @@ var _ = Describe("PostgresReconciler", func() { It("should not try to delete roles or database", func() { // Neither DropRole nor DropDatabase should be called - pg.EXPECT().DropRole(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) - pg.EXPECT().DropDatabase(gomock.Any(), gomock.Any()).Times(0) + pg.EXPECT().DropRole(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + pg.EXPECT().DropDatabase(gomock.Any()).Times(0) // Call Reconcile err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) }) - }) Context("DropOnDelete is enabled", func() { - var ( dropGroupRole *gomock.Call dropReaderRole *gomock.Call @@ -215,10 +208,10 @@ var _ = Describe("PostgresReconciler", func() { BeforeEach(func() { // Expected function calls pg.EXPECT().GetUser().Return("pguser").AnyTimes() - dropGroupRole = pg.EXPECT().DropRole(name+"-owner", "pguser", name, gomock.Any()) - dropReaderRole = pg.EXPECT().DropRole(name+"-reader", "pguser", name, gomock.Any()) - dropWriterRole = pg.EXPECT().DropRole(name+"-writer", "pguser", name, gomock.Any()) - dropDatabase = pg.EXPECT().DropDatabase(name, gomock.Any()) + dropGroupRole = pg.EXPECT().DropRole(name+"-owner", "pguser", name) + dropReaderRole = pg.EXPECT().DropRole(name+"-reader", "pguser", name) + dropWriterRole = pg.EXPECT().DropRole(name+"-writer", "pguser", name) + dropDatabase = pg.EXPECT().DropDatabase(name) // Create Postgres with DropOnDelete == true anotherPostgres := postgresCR.DeepCopy() anotherPostgres.Spec.DropOnDelete = true @@ -228,7 +221,6 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Deletion is successful", func() { - It("should remove finalizer", func() { // No method should return error dropGroupRole.Return(nil) @@ -240,7 +232,7 @@ var _ = Describe("PostgresReconciler", func() { // Call Reconcile err := runReconcile(rp, ctx, req) // Patching both the object and its status fails when using the the FakeClient - //if testEnv != nil { + // if testEnv != nil { Expect(err).NotTo(HaveOccurred()) // Check updated Postgres @@ -251,13 +243,10 @@ var _ = Describe("PostgresReconciler", func() { Expect(foundPostgres.GetFinalizers()).To(BeEmpty()) } //} - }) - }) Context("Deletion is not successful", func() { - It("should not remove finalizer when any database action fails", func() { // DropDatabase fails dropDatabase.Return(fmt.Errorf("Could not drop database")) @@ -270,7 +259,6 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.GetFinalizers()).To(ConsistOf("finalizer.db.movetokube.com")) }) - }) Context("Another Postgres exists with same database", func() { @@ -314,11 +302,8 @@ var _ = Describe("PostgresReconciler", func() { err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) }) - }) - }) - }) Describe("Checking creation logic", func() { @@ -338,7 +323,6 @@ var _ = Describe("PostgresReconciler", func() { }) Context("MasterRole is unset", func() { - BeforeEach(func() { initClient(postgresCR, false) }) @@ -355,11 +339,9 @@ var _ = Describe("PostgresReconciler", func() { err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) }) - }) Context("MasterRole is set", func() { - BeforeEach(func() { // Create client modPostgres := postgresCR.DeepCopy() @@ -379,11 +361,9 @@ var _ = Describe("PostgresReconciler", func() { err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) }) - }) Context("Correct annotation filter is set", func() { - BeforeEach(func() { // Create client modPostgres := postgresCR.DeepCopy() @@ -409,7 +389,6 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Incorrect annotation filter is set", func() { - BeforeEach(func() { // Create client modPostgres := postgresCR.DeepCopy() @@ -427,7 +406,6 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Creation is successful", func() { - BeforeEach(func() { initClient(postgresCR, false) // Expected function calls @@ -462,11 +440,9 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.GetFinalizers()).To(ContainElement(expectedFinalizer)) }) - }) Context("Creation is not successful", func() { - BeforeEach(func() { initClient(postgresCR.DeepCopy(), false) // Expected function calls @@ -489,9 +465,7 @@ var _ = Describe("PostgresReconciler", func() { Expect(foundPostgres.Status.Roles).To(Equal(expectedRoles)) Expect(foundPostgres.Status.Succeeded).To(BeFalse()) }) - }) - }) Describe("Checking extensions logic", func() { @@ -513,14 +487,13 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Postgres has no extensions", func() { - BeforeEach(func() { initClient(postgresCR, false) }) It("should not try to create extensions", func() { // CreateExtension should not be called - pg.EXPECT().CreateExtension(name, gomock.Any(), gomock.Any()).Times(0) + pg.EXPECT().CreateExtension(name, gomock.Any()).Times(0) // Call Reconcile err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) @@ -535,11 +508,9 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Extensions).To(BeEmpty()) }) - }) Context("Postgres has extensions", func() { - BeforeEach(func() { // Add extensions to Postgres object extPostgres := postgresCR.DeepCopy() @@ -548,11 +519,10 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Creation is successful", func() { - BeforeEach(func() { // Expected method calls - pg.EXPECT().CreateExtension(name, "pg_stat_statements", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().CreateExtension(name, "hstore", gomock.Any()).Return(nil).Times(1) + pg.EXPECT().CreateExtension(name, "pg_stat_statements").Return(nil).Times(1) + pg.EXPECT().CreateExtension(name, "hstore").Return(nil).Times(1) }) It("should update status", func() { @@ -564,15 +534,13 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Extensions).To(ConsistOf("pg_stat_statements", "hstore")) }) - }) Context("Creation is not successful", func() { - BeforeEach(func() { // Expected method calls - pg.EXPECT().CreateExtension(name, "pg_stat_statements", gomock.Any()).Return(fmt.Errorf("Could not create extension")).Times(1) - pg.EXPECT().CreateExtension(name, "hstore", gomock.Any()).Return(nil).Times(1) + pg.EXPECT().CreateExtension(name, "pg_stat_statements").Return(fmt.Errorf("Could not create extension")).Times(1) + pg.EXPECT().CreateExtension(name, "hstore").Return(nil).Times(1) }) It("should update status", func() { @@ -584,13 +552,10 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Extensions).To(ConsistOf("hstore")) }) - }) - }) Context("Subset of extensions already created", func() { - BeforeEach(func() { // Add extensions to Postgres object extPostgres := postgresCR.DeepCopy() @@ -600,11 +565,10 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Creation is successful", func() { - It("should not recreate existing extension", func() { // Expected method calls - pg.EXPECT().CreateExtension(name, "pg_stat_statements", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().CreateExtension(name, "hstore", gomock.Any()).Times(0) + pg.EXPECT().CreateExtension(name, "pg_stat_statements").Return(nil).Times(1) + pg.EXPECT().CreateExtension(name, "hstore").Times(0) // Call reconcile err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) @@ -613,11 +577,8 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Extensions).To(ConsistOf("hstore", "pg_stat_statements")) }) - }) - }) - }) Describe("Checking schemas logic", func() { @@ -644,14 +605,13 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Postgres has no schemas", func() { - BeforeEach(func() { initClient(postgresCR, false) }) It("should not try to create schemas", func() { // CreateSchema should not be called - pg.EXPECT().CreateSchema(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + pg.EXPECT().CreateSchema(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) // Call Reconcile err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) @@ -666,11 +626,9 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Schemas).To(BeEmpty()) }) - }) Context("Postgres has schemas", func() { - BeforeEach(func() { // Add schemas to Postgres object schemaPostgres := postgresCR.DeepCopy() @@ -679,15 +637,14 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Creation is successful", func() { - BeforeEach(func() { // Expected method calls // customers schema - pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(gomock.Any(), gomock.Any()).Return(nil).Times(3) + pg.EXPECT().CreateSchema(name, name+"-group", "customers").Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(gomock.Any()).Return(nil).Times(3) // stores schema - pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(gomock.Any(), gomock.Any()).Return(nil).Times(3) + pg.EXPECT().CreateSchema(name, name+"-group", "stores").Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(gomock.Any()).Return(nil).Times(3) }) It("should update status", func() { @@ -699,19 +656,17 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Schemas).To(ConsistOf("customers", "stores")) }) - }) Context("Creation is not successful", func() { - BeforeEach(func() { // Expected method calls // customers schema errors - pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(fmt.Errorf("Could not create schema")).Times(1) - pg.EXPECT().SetSchemaPrivileges(gomock.Any(), gomock.Any()).Return(nil).Times(0) + pg.EXPECT().CreateSchema(name, name+"-group", "customers").Return(fmt.Errorf("Could not create schema")).Times(1) + pg.EXPECT().SetSchemaPrivileges(gomock.Any()).Return(nil).Times(0) // stores schema - pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(gomock.Any(), gomock.Any()).Return(nil).Times(3) + pg.EXPECT().CreateSchema(name, name+"-group", "stores").Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(gomock.Any()).Return(nil).Times(3) }) It("should update status", func() { @@ -723,13 +678,10 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Schemas).To(ConsistOf("stores")) }) - }) - }) Context("Subset of schema already created", func() { - BeforeEach(func() { // Add schemas to Postgres object schemaPostgres := postgresCR.DeepCopy() @@ -739,14 +691,13 @@ var _ = Describe("PostgresReconciler", func() { }) Context("Creation is successful", func() { - It("should not recreate existing schema", func() { // customers schema - pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(gomock.Any(), gomock.Any()).Return(nil).Times(3) + pg.EXPECT().CreateSchema(name, name+"-group", "customers").Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(gomock.Any()).Return(nil).Times(3) // stores schema already exists - pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Times(0) - pg.EXPECT().SetSchemaPrivileges(gomock.Any(), gomock.Any()).Return(nil).Times(0) + pg.EXPECT().CreateSchema(name, name+"-group", "stores").Times(0) + pg.EXPECT().SetSchemaPrivileges(gomock.Any()).Return(nil).Times(0) // Call reconcile err := runReconcile(rp, ctx, req) Expect(err).NotTo(HaveOccurred()) @@ -755,11 +706,7 @@ var _ = Describe("PostgresReconciler", func() { Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) Expect(foundPostgres.Status.Schemas).To(ConsistOf("stores", "customers")) }) - }) - }) - }) - }) diff --git a/internal/controller/postgresuser_controller.go b/internal/controller/postgresuser_controller.go index 6870a683d..337134b89 100644 --- a/internal/controller/postgresuser_controller.go +++ b/internal/controller/postgresuser_controller.go @@ -33,7 +33,7 @@ type PostgresUserReconciler struct { pgUriArgs string instanceFilter string keepSecretName bool // use secret name as defined in PostgresUserSpec - cloudProvider string + cloudProvider config.CloudProvider } // NewPostgresUserReconciler returns a new reconcile.Reconciler @@ -99,8 +99,7 @@ func (r *PostgresUserReconciler) Reconcile(ctx context.Context, req ctrl.Request if postgres != nil && postgres.GetDeletionTimestamp().IsZero() { db = instance.Status.DatabaseName } - err = r.pg.DropRole(instance.Status.PostgresRole, instance.Status.PostgresGroup, - db, reqLogger) + err = r.pg.DropRole(instance.Status.PostgresRole, instance.Status.PostgresGroup, db) if err != nil { return ctrl.Result{}, err } @@ -118,7 +117,6 @@ func (r *PostgresUserReconciler) Reconcile(ctx context.Context, req ctrl.Request // Creation logic var role, login string password, err := utils.GetSecureRandomString(15) - if err != nil { return r.requeue(ctx, instance, err) } @@ -176,7 +174,7 @@ func (r *PostgresUserReconciler) Reconcile(ctx context.Context, req ctrl.Request awsConfig := instance.Spec.AWS awsIamRequested := awsConfig != nil && awsConfig.EnableIamAuth - if r.cloudProvider == "AWS" { + if r.cloudProvider == config.CloudProviderAWS { if awsIamRequested && !instance.Status.EnableIamAuth { if err := r.pg.GrantRole("rds_iam", role); err != nil { reqLogger.WithValues("role", role).Error(err, "failed to grant rds_iam role") diff --git a/internal/controller/postgresuser_controller_test.go b/internal/controller/postgresuser_controller_test.go index 543671da4..87072f6ec 100644 --- a/internal/controller/postgresuser_controller_test.go +++ b/internal/controller/postgresuser_controller_test.go @@ -189,7 +189,7 @@ var _ = Describe("PostgresUser Controller", func() { // Expect DropRole to be called pg.EXPECT().GetDefaultDatabase().Return("postgres") pg.EXPECT().DropRole(postgresUser.Status.PostgresRole, postgresUser.Status.PostgresGroup, - databaseName, gomock.Any()).Return(nil) + databaseName).Return(nil) // Call Reconcile err := runReconcile(rp, ctx, req) @@ -209,7 +209,7 @@ var _ = Describe("PostgresUser Controller", func() { // Expect DropRole to fail pg.EXPECT().GetDefaultDatabase().Return("postgres") pg.EXPECT().DropRole(postgresUser.Status.PostgresRole, postgresUser.Status.PostgresGroup, - databaseName, gomock.Any()).Return(fmt.Errorf("failed to drop role")) + databaseName).Return(fmt.Errorf("failed to drop role")) // Call Reconcile err := runReconcile(rp, ctx, req) Expect(err).To(HaveOccurred()) diff --git a/pkg/config/config.go b/pkg/config/config.go index 5bd44a45d..5b63497c2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -3,6 +3,7 @@ package config import ( "net/url" "strconv" + "strings" "sync" "github.com/movetokube/postgres-operator/pkg/utils" @@ -14,13 +15,24 @@ type Cfg struct { PostgresPass string PostgresUriArgs string PostgresDefaultDb string - CloudProvider string + CloudProvider CloudProvider AnnotationFilter string KeepSecretName bool } -var doOnce sync.Once -var config *Cfg +var ( + doOnce sync.Once + config *Cfg +) + +type CloudProvider string + +const ( + CloudProviderNone CloudProvider = "None" + CloudProviderAWS CloudProvider = "AWS" + CloudProviderAzure CloudProvider = "Azure" + CloudProviderGCP CloudProvider = "GCP" +) func Get() *Cfg { doOnce.Do(func() { @@ -30,7 +42,7 @@ func Get() *Cfg { config.PostgresPass = url.PathEscape(utils.MustGetEnv("POSTGRES_PASS")) config.PostgresUriArgs = utils.MustGetEnv("POSTGRES_URI_ARGS") config.PostgresDefaultDb = utils.GetEnv("POSTGRES_DEFAULT_DATABASE") - config.CloudProvider = utils.GetEnv("POSTGRES_CLOUD_PROVIDER") + config.CloudProvider = ParseCloudProvider(utils.GetEnv("POSTGRES_CLOUD_PROVIDER")) config.AnnotationFilter = utils.GetEnv("POSTGRES_INSTANCE") if value, err := strconv.ParseBool(utils.GetEnv("KEEP_SECRET_NAME")); err == nil { config.KeepSecretName = value @@ -38,3 +50,18 @@ func Get() *Cfg { }) return config } + +// CloudProvider is an enum for supported cloud providers. + +func ParseCloudProvider(s string) CloudProvider { + switch strings.ToLower(s) { + case "aws": + return CloudProviderAWS + case "azure": + return CloudProviderAzure + case "gcp": + return CloudProviderGCP + default: + return CloudProviderNone + } +} diff --git a/pkg/postgres/aws.go b/pkg/postgres/aws.go index 61e732357..a27167a4d 100644 --- a/pkg/postgres/aws.go +++ b/pkg/postgres/aws.go @@ -3,7 +3,6 @@ package postgres import ( "fmt" - "github.com/go-logr/logr" "github.com/lib/pq" ) @@ -54,7 +53,7 @@ func (c *awspg) CreateUserRole(role, password string) (string, error) { return returnedRole, nil } -func (c *awspg) DropRole(role, newOwner, database string, logger logr.Logger) error { +func (c *awspg) DropRole(role, newOwner, database string) error { // On AWS RDS the postgres user isn't really superuser so he doesn't have permissions // to REASSIGN OWNED BY unless he belongs to both roles err := c.GrantRole(role, c.user) @@ -69,12 +68,12 @@ func (c *awspg) DropRole(role, newOwner, database string, logger logr.Logger) er if err != nil && err.(*pq.Error).Code != "0LP01" { if err.(*pq.Error).Code == "42704" { // The group role does not exist, no point of granting roles - logger.Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner)) + c.log.Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner)) return nil } return err } defer c.RevokeRole(newOwner, c.user) - return c.pg.DropRole(role, newOwner, database, logger) + return c.pg.DropRole(role, newOwner, database) } diff --git a/pkg/postgres/azure.go b/pkg/postgres/azure.go index 99628bcb7..a87696a0c 100644 --- a/pkg/postgres/azure.go +++ b/pkg/postgres/azure.go @@ -1,7 +1,6 @@ package postgres import ( - "github.com/go-logr/logr" "github.com/lib/pq" ) @@ -35,7 +34,7 @@ func (azpg *azurepg) CreateDB(dbname, role string) error { return azpg.pg.CreateDB(dbname, role) } -func (azpg *azurepg) DropRole(role, newOwner, database string, logger logr.Logger) error { +func (azpg *azurepg) DropRole(role, newOwner, database string) error { // Grant the role to the user first err := azpg.GrantRole(role, azpg.user) if err != nil && err.(*pq.Error).Code != "0LP01" { @@ -46,5 +45,5 @@ func (azpg *azurepg) DropRole(role, newOwner, database string, logger logr.Logge } // Delegate to parent implementation to perform the actual drop - return azpg.pg.DropRole(role, newOwner, database, logger) + return azpg.pg.DropRole(role, newOwner, database) } diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index a6b260f57..b9d30c3a8 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -3,7 +3,6 @@ package postgres import ( "fmt" - "github.com/go-logr/logr" "github.com/lib/pq" ) @@ -44,8 +43,8 @@ func (c *pg) CreateDB(dbname, role string) error { return nil } -func (c *pg) CreateSchema(db, role, schema string, logger logr.Logger) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args, logger) +func (c *pg) CreateSchema(db, role, schema string) error { + tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args) if err != nil { return err } @@ -58,7 +57,7 @@ func (c *pg) CreateSchema(db, role, schema string, logger logr.Logger) error { return nil } -func (c *pg) DropDatabase(database string, logger logr.Logger) error { +func (c *pg) DropDatabase(database string) error { _, err := c.db.Exec(fmt.Sprintf(REVOKE_CONNECT, database)) // Error code 3D000 is returned if database doesn't exist if err != nil && err.(*pq.Error).Code != "3D000" { @@ -76,13 +75,13 @@ func (c *pg) DropDatabase(database string, logger logr.Logger) error { return err } - logger.Info(fmt.Sprintf("Dropped database %s", database)) + c.log.Info(fmt.Sprintf("Dropped database %s", database)) return nil } -func (c *pg) CreateExtension(db, extension string, logger logr.Logger) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args, logger) +func (c *pg) CreateExtension(db, extension string) error { + tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args) if err != nil { return err } @@ -95,8 +94,8 @@ func (c *pg) CreateExtension(db, extension string, logger logr.Logger) error { return nil } -func (c *pg) SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges, logger logr.Logger) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, schemaPrivileges.DB, c.args, logger) +func (c *pg) SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges) error { + tmpDb, err := GetConnection(c.user, c.pass, c.host, schemaPrivileges.DB, c.args) if err != nil { return err } diff --git a/pkg/postgres/gcp.go b/pkg/postgres/gcp.go index 1531ffb84..837ab6952 100644 --- a/pkg/postgres/gcp.go +++ b/pkg/postgres/gcp.go @@ -3,7 +3,6 @@ package postgres import ( "fmt" - "github.com/go-logr/logr" "github.com/lib/pq" ) @@ -17,8 +16,7 @@ func newGCPPG(postgres *pg) PG { } } -func (c *gcppg) DropDatabase(database string, logger logr.Logger) error { - +func (c *gcppg) DropDatabase(database string) error { _, err := c.db.Exec(fmt.Sprintf(REVOKE_CONNECT, database)) // Error code 3D000 is returned if database doesn't exist if err != nil && err.(*pq.Error).Code != "3D000" { @@ -36,13 +34,12 @@ func (c *gcppg) DropDatabase(database string, logger logr.Logger) error { return err } - logger.Info(fmt.Sprintf("Dropped database %s", database)) + c.log.Info(fmt.Sprintf("Dropped database %s", database)) return nil } func (c *gcppg) CreateDB(dbname, role string) error { - err := c.GrantRole(role, c.user) if err != nil { return err @@ -54,11 +51,10 @@ func (c *gcppg) CreateDB(dbname, role string) error { return nil } -func (c *gcppg) DropRole(role, newOwner, database string, logger logr.Logger) error { - - tmpDb, err := GetConnection(c.user, c.pass, c.host, database, c.args, logger) +func (c *gcppg) DropRole(role, newOwner, database string) error { + tmpDb, err := GetConnection(c.user, c.pass, c.host, database, c.args) q := fmt.Sprintf(GET_DB_OWNER, database) - logger.Info("Checking master role: " + q) + c.log.V(1).Info("Checking master role: " + q) rows, err := tmpDb.Query(q) if err != nil { return err @@ -70,7 +66,7 @@ func (c *gcppg) DropRole(role, newOwner, database string, logger logr.Logger) er if role != masterRole { q = fmt.Sprintf(DROP_ROLE, role) - logger.Info("GCP Drop Role: " + q) + c.log.V(1).Info("GCP Drop Role: " + q) _, err = tmpDb.Exec(q) // Check if error exists and if different from "ROLE NOT FOUND" => 42704 if err != nil && err.(*pq.Error).Code != "42704" { @@ -79,7 +75,7 @@ func (c *gcppg) DropRole(role, newOwner, database string, logger logr.Logger) er defer tmpDb.Close() } else { - logger.Info(fmt.Sprintf("GCP refusing DropRole on master role: %s", masterRole)) + c.log.Info(fmt.Sprintf("GCP refusing DropRole on master role: %s", masterRole)) } return nil } diff --git a/pkg/postgres/mock/postgres.go b/pkg/postgres/mock/postgres.go index 4f53e59ec..d9b70dc0d 100644 --- a/pkg/postgres/mock/postgres.go +++ b/pkg/postgres/mock/postgres.go @@ -12,7 +12,6 @@ package mock_postgres import ( reflect "reflect" - logr "github.com/go-logr/logr" postgres "github.com/movetokube/postgres-operator/pkg/postgres" gomock "go.uber.org/mock/gomock" ) @@ -70,17 +69,17 @@ func (mr *MockPGMockRecorder) CreateDB(dbname, username any) *gomock.Call { } // CreateExtension mocks base method. -func (m *MockPG) CreateExtension(db, extension string, logger logr.Logger) error { +func (m *MockPG) CreateExtension(db, extension string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateExtension", db, extension, logger) + ret := m.ctrl.Call(m, "CreateExtension", db, extension) ret0, _ := ret[0].(error) return ret0 } // CreateExtension indicates an expected call of CreateExtension. -func (mr *MockPGMockRecorder) CreateExtension(db, extension, logger any) *gomock.Call { +func (mr *MockPGMockRecorder) CreateExtension(db, extension any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateExtension", reflect.TypeOf((*MockPG)(nil).CreateExtension), db, extension, logger) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateExtension", reflect.TypeOf((*MockPG)(nil).CreateExtension), db, extension) } // CreateGroupRole mocks base method. @@ -98,17 +97,17 @@ func (mr *MockPGMockRecorder) CreateGroupRole(role any) *gomock.Call { } // CreateSchema mocks base method. -func (m *MockPG) CreateSchema(db, role, schema string, logger logr.Logger) error { +func (m *MockPG) CreateSchema(db, role, schema string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateSchema", db, role, schema, logger) + ret := m.ctrl.Call(m, "CreateSchema", db, role, schema) ret0, _ := ret[0].(error) return ret0 } // CreateSchema indicates an expected call of CreateSchema. -func (mr *MockPGMockRecorder) CreateSchema(db, role, schema, logger any) *gomock.Call { +func (mr *MockPGMockRecorder) CreateSchema(db, role, schema any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateSchema", reflect.TypeOf((*MockPG)(nil).CreateSchema), db, role, schema, logger) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateSchema", reflect.TypeOf((*MockPG)(nil).CreateSchema), db, role, schema) } // CreateUserRole mocks base method. @@ -127,31 +126,31 @@ func (mr *MockPGMockRecorder) CreateUserRole(role, password any) *gomock.Call { } // DropDatabase mocks base method. -func (m *MockPG) DropDatabase(db string, logger logr.Logger) error { +func (m *MockPG) DropDatabase(db string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropDatabase", db, logger) + ret := m.ctrl.Call(m, "DropDatabase", db) ret0, _ := ret[0].(error) return ret0 } // DropDatabase indicates an expected call of DropDatabase. -func (mr *MockPGMockRecorder) DropDatabase(db, logger any) *gomock.Call { +func (mr *MockPGMockRecorder) DropDatabase(db any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropDatabase", reflect.TypeOf((*MockPG)(nil).DropDatabase), db, logger) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropDatabase", reflect.TypeOf((*MockPG)(nil).DropDatabase), db) } // DropRole mocks base method. -func (m *MockPG) DropRole(role, newOwner, database string, logger logr.Logger) error { +func (m *MockPG) DropRole(role, newOwner, database string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropRole", role, newOwner, database, logger) + ret := m.ctrl.Call(m, "DropRole", role, newOwner, database) ret0, _ := ret[0].(error) return ret0 } // DropRole indicates an expected call of DropRole. -func (mr *MockPGMockRecorder) DropRole(role, newOwner, database, logger any) *gomock.Call { +func (mr *MockPGMockRecorder) DropRole(role, newOwner, database any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropRole", reflect.TypeOf((*MockPG)(nil).DropRole), role, newOwner, database, logger) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropRole", reflect.TypeOf((*MockPG)(nil).DropRole), role, newOwner, database) } // GetDefaultDatabase mocks base method. @@ -211,17 +210,17 @@ func (mr *MockPGMockRecorder) RevokeRole(role, revoked any) *gomock.Call { } // SetSchemaPrivileges mocks base method. -func (m *MockPG) SetSchemaPrivileges(schemaPrivileges postgres.PostgresSchemaPrivileges, logger logr.Logger) error { +func (m *MockPG) SetSchemaPrivileges(schemaPrivileges postgres.PostgresSchemaPrivileges) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetSchemaPrivileges", schemaPrivileges, logger) + ret := m.ctrl.Call(m, "SetSchemaPrivileges", schemaPrivileges) ret0, _ := ret[0].(error) return ret0 } // SetSchemaPrivileges indicates an expected call of SetSchemaPrivileges. -func (mr *MockPGMockRecorder) SetSchemaPrivileges(schemaPrivileges, logger any) *gomock.Call { +func (mr *MockPGMockRecorder) SetSchemaPrivileges(schemaPrivileges any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivileges", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivileges), schemaPrivileges, logger) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivileges", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivileges), schemaPrivileges) } // UpdatePassword mocks base method. diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index 033640abe..1bdb6c9ce 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -3,7 +3,6 @@ package postgres import ( "database/sql" "fmt" - "log" "github.com/go-logr/logr" "github.com/movetokube/postgres-operator/pkg/config" @@ -11,29 +10,29 @@ import ( type PG interface { CreateDB(dbname, username string) error - CreateSchema(db, role, schema string, logger logr.Logger) error - CreateExtension(db, extension string, logger logr.Logger) error + CreateSchema(db, role, schema string) error + CreateExtension(db, extension string) error CreateGroupRole(role string) error CreateUserRole(role, password string) (string, error) UpdatePassword(role, password string) error GrantRole(role, grantee string) error - SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges, logger logr.Logger) error + SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges) error RevokeRole(role, revoked string) error AlterDefaultLoginRole(role, setRole string) error - DropDatabase(db string, logger logr.Logger) error - DropRole(role, newOwner, database string, logger logr.Logger) error + DropDatabase(db string) error + DropRole(role, newOwner, database string) error GetUser() string GetDefaultDatabase() string } type pg struct { - db *sql.DB - log logr.Logger - host string - user string - pass string - args string - default_database string + db *sql.DB + log logr.Logger + host string + user string + pass string + args string + defaultDatabase string } type PostgresSchemaPrivileges struct { @@ -50,30 +49,29 @@ func NewPG(cfg *config.Cfg, logger logr.Logger) (PG, error) { cfg.PostgresPass, cfg.PostgresHost, cfg.PostgresDefaultDb, - cfg.PostgresUriArgs, - logger) + cfg.PostgresUriArgs) if err != nil { - log.Fatalf("failed to connect to PostgreSQL server: %s", err.Error()) + return nil, err } - logger.Info("connected to postgres server") + logger.V(1).Info("connected to postgres server") postgres := &pg{ - db: db, - log: logger, - host: cfg.PostgresHost, - user: cfg.PostgresUser, - pass: cfg.PostgresPass, - args: cfg.PostgresUriArgs, - default_database: cfg.PostgresDefaultDb, + db: db, + log: logger, + host: cfg.PostgresHost, + user: cfg.PostgresUser, + pass: cfg.PostgresPass, + args: cfg.PostgresUriArgs, + defaultDatabase: cfg.PostgresDefaultDb, } switch cfg.CloudProvider { - case "AWS": + case config.CloudProviderAWS: logger.Info("Using AWS wrapper") return newAWSPG(postgres), nil - case "Azure": + case config.CloudProviderAzure: logger.Info("Using Azure wrapper") return newAzurePG(postgres), nil - case "GCP": + case config.CloudProviderGCP: logger.Info("Using GCP wrapper") return newGCPPG(postgres), nil default: @@ -87,13 +85,13 @@ func (c *pg) GetUser() string { } func (c *pg) GetDefaultDatabase() string { - return c.default_database + return c.defaultDatabase } -func GetConnection(user, password, host, database, uri_args string, logger logr.Logger) (*sql.DB, error) { - db, err := sql.Open("postgres", fmt.Sprintf("postgresql://%s:%s@%s/%s?%s", user, password, host, database, uri_args)) +func GetConnection(user, password, host, database, uriArgs string) (*sql.DB, error) { + db, err := sql.Open("postgres", fmt.Sprintf("postgresql://%s:%s@%s/%s?%s", user, password, host, database, uriArgs)) if err != nil { - log.Fatal(err) + return nil, err } err = db.Ping() return db, err diff --git a/pkg/postgres/role.go b/pkg/postgres/role.go index 8bf4f4b71..c2e941bb4 100644 --- a/pkg/postgres/role.go +++ b/pkg/postgres/role.go @@ -3,7 +3,6 @@ package postgres import ( "fmt" - "github.com/go-logr/logr" "github.com/lib/pq" ) @@ -60,9 +59,9 @@ func (c *pg) RevokeRole(role, revoked string) error { return nil } -func (c *pg) DropRole(role, newOwner, database string, logger logr.Logger) error { +func (c *pg) DropRole(role, newOwner, database string) error { // REASSIGN OWNED BY only works if the correct database is selected - tmpDb, err := GetConnection(c.user, c.pass, c.host, database, c.args, logger) + tmpDb, err := GetConnection(c.user, c.pass, c.host, database, c.args) if err != nil { if err.(*pq.Error).Code == "3D000" { return nil // Database is does not exist (anymore)