Add MySQLUser and MySQLAADUser v1alpha2 (#1357)

* Add v1alpha2 MySQLUser This removes DbName from MySQLUser and adds DatabaseRoles to store per-database permissions. Roles will now only store server-wide permissions. Add conversions between v1alpha1 and v1alpha2 versions. * Add v1alpha2 MySQLAADUser This removes DBName from MySQLAADUser and adds DatabaseRoles to store per-database permissions. Roles will now only store server-wide permissions. Add conversions between v1alpha1 and v1alpha2 versions. * Set up conversion webhooks for MySQLUser and MySQLAADUser * Review feedback, thanks @matthchr! * Ensure `preserveUnknownFields: false` is set in all webhook patches These were set for all types with version conversions but not the others (which aren't in use since they are still commented out in kustomization.yaml). Turning them on in the rest to remove one step in the process of adding conversion webhooks to types in the future. This setting is required for conversion to work - it seems like the only reason it's not set in the patches is that they were generated by kubebuilder before the setting was mandatory. * Add provisioning state methods to v1alpha2 ASOStatus * Reimplement MySQLUser and MySQLAADUser reconciliation with v1alpha2 They now check server-level (in USER_PRIVILEGES) and database-level (SCHEMA_PRIVILEGES) permissions. * Update controller tests to work with v1alpha2 MySQLUser * Move system database constant to mysql.SystemDatabase Also rename the ServerPort and DriverName constants so they don't repeat the name of the package. * Change EnsureUserDatabaseRoles to return just an error And change the reconciliation code for user and aad user to just treat that as a provisioning failure, rather than saying that it had succeeded but there were some errors which is just confusing. We still try to apply changes to all databases even if there is an error for one of them. Also other review changes, thanks @matthchr!
Azure · Feb 1, 2021 · 7991802 · 7991802
1 parent c446f1d
commit 7991802
Show file tree

Hide file tree

Showing 60 changed files with 1,283 additions and 160 deletions.
diff --git a/api/v1alpha1/mysqlaaduser_conversion.go b/api/v1alpha1/mysqlaaduser_conversion.go
@@ -0,0 +1,72 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package v1alpha1
+
+import (
+	"github.com/pkg/errors"
+	"sigs.k8s.io/controller-runtime/pkg/conversion"
+
+	"github.com/Azure/azure-service-operator/api/v1alpha2"
+)
+
+var _ conversion.Convertible = &MySQLAADUser{}
+
+func (src *MySQLAADUser) ConvertTo(dstRaw conversion.Hub) error {
+	dst := dstRaw.(*v1alpha2.MySQLAADUser)
+
+	// ObjectMeta
+	dst.ObjectMeta = src.ObjectMeta
+
+	// Spec
+	dst.Spec.ResourceGroup = src.Spec.ResourceGroup
+	dst.Spec.Server = src.Spec.Server
+	dst.Spec.AADID = src.Spec.AADID
+	dst.Spec.Username = src.Spec.Username
+
+	// v1alpha1 doesn't support server-level roles, only
+	// database-level ones, so we move them into the new field.
+	dst.Spec.Roles = []string{}
+	dst.Spec.DatabaseRoles = make(map[string][]string)
+	dst.Spec.DatabaseRoles[src.Spec.DBName] = append([]string(nil), src.Spec.Roles...)
+
+	// Status
+	dst.Status = v1alpha2.ASOStatus(src.Status)
+
+	return nil
+}
+
+func (dst *MySQLAADUser) ConvertFrom(srcRaw conversion.Hub) error {
+	src := srcRaw.(*v1alpha2.MySQLAADUser)
+
+	// Converting a v1alpha2 user into a v1alpha1 one is only allowed
+	// if it has exactly one database...
+	if len(src.Spec.DatabaseRoles) != 1 {
+		return errors.Errorf("can't convert user %q to %T because it has privileges in %d databases", src.ObjectMeta.Name, dst, len(src.Spec.DatabaseRoles))
+	}
+	// ...and no server-level roles.
+	if len(src.Spec.Roles) != 0 {
+		return errors.Errorf("can't convert user %q to %T because it has server-level roles", src.ObjectMeta.Name, dst)
+	}
+
+	// ObjectMeta
+	dst.ObjectMeta = src.ObjectMeta
+
+	// Spec
+	dst.Spec.ResourceGroup = src.Spec.ResourceGroup
+	dst.Spec.Server = src.Spec.Server
+	dst.Spec.AADID = src.Spec.AADID
+	dst.Spec.Username = src.Spec.Username
+
+	for dbName, roles := range src.Spec.DatabaseRoles {
+		dst.Spec.DBName = dbName
+		dst.Spec.Roles = append(dst.Spec.Roles, roles...)
+		break
+	}
+
+	// Status
+	dst.Status = ASOStatus(src.Status)
+
+	return nil
+
+}
diff --git a/api/v1alpha1/mysqlaaduser_types_test.go b/api/v1alpha1/mysqlaaduser_types_test.go
@@ -0,0 +1,145 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package v1alpha1
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/Azure/azure-service-operator/api/v1alpha2"
+)
+
+var _ = Describe("MySQLAADUser", func() {
+	Context("Conversion", func() {
+		It("can upgrade to v1alpha2", func() {
+
+			v1 := &MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: MySQLAADUserSpec{
+					Server:        "myserver",
+					DBName:        "mydb",
+					ResourceGroup: "foo-group",
+					Roles:         []string{"role1", "role2", "role3"},
+					AADID:         "aadid",
+					Username:      "username",
+				},
+			}
+
+			var v2 v1alpha2.MySQLAADUser
+			Expect(v1.ConvertTo(&v2)).To(Succeed())
+			Expect(v2).To(Equal(v1alpha2.MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: v1alpha2.MySQLAADUserSpec{
+					Server:        "myserver",
+					ResourceGroup: "foo-group",
+					Roles:         []string{},
+					DatabaseRoles: map[string][]string{
+						"mydb": {"role1", "role2", "role3"},
+					},
+					AADID:    "aadid",
+					Username: "username",
+				},
+			}))
+		})
+
+		It("can downgrade to v1alpha1 if it's the right shape", func() {
+			v2 := v1alpha2.MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: v1alpha2.MySQLAADUserSpec{
+					Server:        "myserver",
+					ResourceGroup: "foo-group",
+					Roles:         nil,
+					DatabaseRoles: map[string][]string{
+						"mydb": {"role1", "role2", "role3"},
+					},
+					AADID:    "aadid",
+					Username: "username",
+				},
+			}
+			var v1 MySQLAADUser
+			Expect(v1.ConvertFrom(&v2)).To(Succeed())
+			Expect(v1).To(Equal(MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: MySQLAADUserSpec{
+					Server:        "myserver",
+					DBName:        "mydb",
+					ResourceGroup: "foo-group",
+					Roles:         []string{"role1", "role2", "role3"},
+					AADID:         "aadid",
+					Username:      "username",
+				},
+			}))
+		})
+
+		It("can't downgrade with roles for multiple databases", func() {
+			v2 := v1alpha2.MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: v1alpha2.MySQLAADUserSpec{
+					Server:        "myserver",
+					ResourceGroup: "foo-group",
+					DatabaseRoles: map[string][]string{
+						"mydb":    {"role1", "role2", "role3"},
+						"otherdb": {"otherrole"},
+					},
+				},
+			}
+			var v1 MySQLAADUser
+			Expect(v1.ConvertFrom(&v2)).To(MatchError("can't convert user \"foo\" to *v1alpha1.MySQLAADUser because it has privileges in 2 databases"))
+		})
+
+		It("can't downgrade with roles for no databases", func() {
+			v2 := v1alpha2.MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: v1alpha2.MySQLAADUserSpec{
+					Server:        "myserver",
+					ResourceGroup: "foo-group",
+					DatabaseRoles: nil,
+				},
+			}
+			var v1 MySQLAADUser
+			Expect(v1.ConvertFrom(&v2)).To(MatchError("can't convert user \"foo\" to *v1alpha1.MySQLAADUser because it has privileges in 0 databases"))
+		})
+
+		It("can't downgrade with server roles", func() {
+			v2 := v1alpha2.MySQLAADUser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "default",
+				},
+				Spec: v1alpha2.MySQLAADUserSpec{
+					Server:        "myserver",
+					ResourceGroup: "foo-group",
+					Roles:         []string{"somekindofsuperuser"},
+					DatabaseRoles: map[string][]string{
+						"mydb": {"role1", "role2", "role3"},
+					},
+				},
+			}
+			var v1 MySQLAADUser
+			Expect(v1.ConvertFrom(&v2)).To(MatchError("can't convert user \"foo\" to *v1alpha1.MySQLAADUser because it has server-level roles"))
+		})
+
+	})
+
+})
diff --git a/api/v1alpha1/mysqlaaduser_webhook.go b/api/v1alpha1/mysqlaaduser_webhook.go
@@ -0,0 +1,20 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package v1alpha1
+
+import (
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+// log is for logging in this package.
+var mysqlaaduserlog = logf.Log.WithName("mysqlaaduser-resource")
+
+func (r *MySQLAADUser) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(r).
+		Complete()
+}
+
+// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
diff --git a/api/v1alpha1/mysqluser_conversion.go b/api/v1alpha1/mysqluser_conversion.go
@@ -0,0 +1,76 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package v1alpha1
+
+import (
+	"github.com/pkg/errors"
+	"sigs.k8s.io/controller-runtime/pkg/conversion"
+
+	"github.com/Azure/azure-service-operator/api/v1alpha2"
+)
+
+var _ conversion.Convertible = &MySQLAADUser{}
+
+func (src *MySQLUser) ConvertTo(dstRaw conversion.Hub) error {
+	dst := dstRaw.(*v1alpha2.MySQLUser)
+
+	// ObjectMeta
+	dst.ObjectMeta = src.ObjectMeta
+
+	// Spec
+	dst.Spec.ResourceGroup = src.Spec.ResourceGroup
+	dst.Spec.Server = src.Spec.Server
+	dst.Spec.AdminSecret = src.Spec.AdminSecret
+	dst.Spec.AdminSecretKeyVault = src.Spec.AdminSecretKeyVault
+	dst.Spec.Username = src.Spec.Username
+	dst.Spec.KeyVaultToStoreSecrets = src.Spec.KeyVaultToStoreSecrets
+
+	// v1alpha1 doesn't support server-level roles, only
+	// database-level ones, so we move them into the new field.
+	dst.Spec.Roles = []string{}
+	dst.Spec.DatabaseRoles = make(map[string][]string)
+	dst.Spec.DatabaseRoles[src.Spec.DbName] = append([]string(nil), src.Spec.Roles...)
+
+	// Status
+	dst.Status = v1alpha2.ASOStatus(src.Status)
+
+	return nil
+}
+
+func (dst *MySQLUser) ConvertFrom(srcRaw conversion.Hub) error {
+	src := srcRaw.(*v1alpha2.MySQLUser)
+
+	// Converting a v1alpha2 user into a v1alpha1 one is only allowed
+	// if it has exactly one database...
+	if len(src.Spec.DatabaseRoles) != 1 {
+		return errors.Errorf("can't convert user %q to %T because it has privileges in %d databases", src.ObjectMeta.Name, dst, len(src.Spec.DatabaseRoles))
+	}
+	// ...and no server-level roles.
+	if len(src.Spec.Roles) != 0 {
+		return errors.Errorf("can't convert user %q to %T because it has server-level roles", src.ObjectMeta.Name, dst)
+	}
+
+	// ObjectMeta
+	dst.ObjectMeta = src.ObjectMeta
+
+	// Spec
+	dst.Spec.ResourceGroup = src.Spec.ResourceGroup
+	dst.Spec.Server = src.Spec.Server
+	dst.Spec.AdminSecret = src.Spec.AdminSecret
+	dst.Spec.AdminSecretKeyVault = src.Spec.AdminSecretKeyVault
+	dst.Spec.Username = src.Spec.Username
+	dst.Spec.KeyVaultToStoreSecrets = src.Spec.KeyVaultToStoreSecrets
+
+	for dbName, roles := range src.Spec.DatabaseRoles {
+		dst.Spec.DbName = dbName
+		dst.Spec.Roles = append(dst.Spec.Roles, roles...)
+		break
+	}
+
+	// Status
+	dst.Status = ASOStatus(src.Status)
+
+	return nil
+
+}