From 170bdc41d804776c8a68585e250a24e3b5a52e19 Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:00 +0100 Subject: [PATCH 1/6] feat(sql): add AADUserSpec to User CRD types --- v2/api/sql/v1/user_types.go | 25 ++- v2/api/sql/v1/zz_generated.deepcopy.go | 20 ++ v2/config/crd/bases/sql.azure.com_users.yaml | 218 +++++++++++++++++++ 3 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 v2/config/crd/bases/sql.azure.com_users.yaml diff --git a/v2/api/sql/v1/user_types.go b/v2/api/sql/v1/user_types.go index 845c97e643b..c3f724a3dc7 100644 --- a/v2/api/sql/v1/user_types.go +++ b/v2/api/sql/v1/user_types.go @@ -94,9 +94,11 @@ type UserSpec struct { // db_ddladmin, db_datawriter, db_datareader, db_denydatawriter, and db_denydatareader. Roles []string `json:"roles,omitempty"` - // +kubebuilder:validation:Required // LocalUser contains details for creating a standard (non-aad) Azure SQL User LocalUser *LocalUserSpec `json:"localUser,omitempty"` + + // AADUser contains details for creating an AAD user. + AADUser *AADUserSpec `json:"aadUser,omitempty"` } // OriginalVersion returns the original API version used to create the resource. @@ -144,6 +146,27 @@ type LocalUserSpec struct { Password *genruntime.SecretReference `json:"password,omitempty"` } +// AADUserSpec defines the specification for an Azure AD user. +// When creating an AAD user, the AzureName must match the identity name in Azure AD: +// - For managed identity: "my-managed-identity-name" +// - For service principal: "my-app-name" +// - For standard AAD user: "user@domain.onmicrosoft.com" +// - For AAD group: "my-group-name" +type AADUserSpec struct { + // Alias overrides AzureName for the database user name. + // Use when AzureName exceeds 128 characters (Azure SQL Server limit). + // +kubebuilder:validation:MaxLength=128 + Alias string `json:"alias,omitempty"` + + // ServerAdminUsername is the username of the Server administrator. If your server admin was configured with + // Azure Service Operator, this should match the value of the Administrator's $.spec.login field. If the + // administrator is a group, the ServerAdminUsername should be the group name, not the actual username of the + // identity to log in with. For example if the administrator group is "admin-group" and identity "my-identity" is + // a member of that group, the ServerAdminUsername should be "admin-group". + // +kubebuilder:validation:Required + ServerAdminUsername string `json:"serverAdminUsername,omitempty"` +} + type UserStatus struct { // Conditions: The observed state of the resource Conditions []conditions.Condition `json:"conditions,omitempty"` diff --git a/v2/api/sql/v1/zz_generated.deepcopy.go b/v2/api/sql/v1/zz_generated.deepcopy.go index b269f0d51ee..7fe3218274f 100644 --- a/v2/api/sql/v1/zz_generated.deepcopy.go +++ b/v2/api/sql/v1/zz_generated.deepcopy.go @@ -15,6 +15,21 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AADUserSpec) DeepCopyInto(out *AADUserSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AADUserSpec. +func (in *AADUserSpec) DeepCopy() *AADUserSpec { + if in == nil { + return nil + } + out := new(AADUserSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LocalUserSpec) DeepCopyInto(out *LocalUserSpec) { *out = *in @@ -117,6 +132,11 @@ func (in *UserSpec) DeepCopyInto(out *UserSpec) { *out = new(LocalUserSpec) (*in).DeepCopyInto(*out) } + if in.AADUser != nil { + in, out := &in.AADUser, &out.AADUser + *out = new(AADUserSpec) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserSpec. diff --git a/v2/config/crd/bases/sql.azure.com_users.yaml b/v2/config/crd/bases/sql.azure.com_users.yaml new file mode 100644 index 00000000000..ba479e0e3e8 --- /dev/null +++ b/v2/config/crd/bases/sql.azure.com_users.yaml @@ -0,0 +1,218 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.20.0 + name: users.sql.azure.com +spec: + group: sql.azure.com + names: + categories: + - azure + - sql + kind: User + listKind: UserList + plural: users + singular: user + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=='Ready')].status + name: Ready + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].severity + name: Severity + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].reason + name: Reason + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].message + name: Message + type: string + name: v1 + schema: + openAPIV3Schema: + description: User is an Azure SQL user + 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 + 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 + type: string + metadata: + type: object + spec: + properties: + aadUser: + description: AADUser contains details for creating an AAD user. + properties: + alias: + description: |- + Alias overrides AzureName for the database user name. + Use when AzureName exceeds 128 characters (Azure SQL Server limit). + maxLength: 128 + type: string + serverAdminUsername: + description: |- + ServerAdminUsername is the username of the Server administrator. If your server admin was configured with + Azure Service Operator, this should match the value of the Administrator's $.spec.login field. If the + administrator is a group, the ServerAdminUsername should be the group name, not the actual username of the + identity to log in with. For example if the administrator group is "admin-group" and identity "my-identity" is + a member of that group, the ServerAdminUsername should be "admin-group". + type: string + required: + - serverAdminUsername + type: object + azureName: + description: |- + AzureName: The name of the resource in Azure. This is often the same as the name of the resource in Kubernetes but it + doesn't have to be. + If not specified, the default is the name of the Kubernetes object. + When creating a local user, this will be the name of the user created. + When creating an AAD user, this must have a specific format depending on the type of AAD user being created. + For managed identity: "my-managed-identity-name" + For standard AAD user: "myuser@mydomain.onmicrosoft.com" + For AAD group: "my-group" + type: string + localUser: + description: LocalUser contains details for creating a standard (non-aad) + Azure SQL User + properties: + password: + description: Password is the password to use for the user + properties: + key: + description: Key is the key in the Kubernetes secret being + referenced + type: string + name: + description: |- + Name is the name of the Kubernetes secret being referenced. + The secret must be in the same namespace as the resource + type: string + required: + - key + - name + type: object + serverAdminPassword: + description: ServerAdminPassword is a reference to a secret containing + the servers administrator password. + properties: + key: + description: Key is the key in the Kubernetes secret being + referenced + type: string + name: + description: |- + Name is the name of the Kubernetes secret being referenced. + The secret must be in the same namespace as the resource + type: string + required: + - key + - name + type: object + serverAdminUsername: + description: |- + ServerAdminUsername is the username of the Server administrator. If the + administrator is a group, the ServerAdminUsername should be the group name, not the actual username of the + identity to log in with. For example if the administrator group is "admin-group" and identity "my-identity" is + a member of that group, the ServerAdminUsername should be "admin-group". + type: string + required: + - password + - serverAdminPassword + - serverAdminUsername + type: object + owner: + description: |- + Owner: The owner of the resource. The owner controls where the resource goes when it is deployed. The owner also + controls the resources lifecycle. When the owner is deleted the resource will also be deleted. Owner is expected to be a + reference to an sql.azure.com/ServersDatabase resource + properties: + armId: + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + name: + description: This is the name of the Kubernetes resource to reference. + type: string + type: object + roles: + description: |- + The roles assigned to the user. + See https://learn.microsoft.com/sql/relational-databases/security/authentication-access/database-level-roles?view=sql-server-ver16#fixed-database-roles + for the fixed set of roles supported by Azure SQL. + Roles include the following: db_owner, db_securityadmin, db_accessadmin, db_backupoperator, + db_ddladmin, db_datawriter, db_datareader, db_denydatawriter, and db_denydatareader. + items: + type: string + type: array + required: + - owner + type: object + status: + properties: + conditions: + description: 'Conditions: The observed state of the resource' + items: + description: Condition defines an extension to status (an observation) + of a resource + properties: + lastTransitionTime: + description: LastTransitionTime is the last time the condition + transitioned from one status to another. + format: date-time + type: string + message: + description: Message is a human readable message indicating + details about the transition. This field may be empty. + type: string + observedGeneration: + description: |- + ObservedGeneration is the .metadata.generation that the condition was set based upon. For instance, if + .metadata.generation is currently 12, but the .status.condition[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + type: integer + reason: + description: |- + Reason for the condition's last transition. + Reasons are upper CamelCase (PascalCase) with no spaces. A reason is always provided, this field will not be empty. + type: string + severity: + description: |- + Severity with which to treat failures of this type of condition. + For conditions which have positive polarity (Status == True is their normal/healthy state), this will be omitted when Status == True + For conditions which have negative polarity (Status == False is their normal/healthy state), this will be omitted when Status == False. + This is omitted in all cases when Status == Unknown + type: string + status: + description: Status of the condition, one of True, False, or + Unknown. + type: string + type: + description: Type of condition. + type: string + required: + - lastTransitionTime + - reason + - status + - type + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} From f2e5766e9a21c69da2267689c9182f62d965be4d Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:01 +0100 Subject: [PATCH 2/6] feat(sql): add webhook validation for AAD users --- v2/api/sql/v1/webhook/user_webhook_types.go | 63 +++++++++++++++++++-- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/v2/api/sql/v1/webhook/user_webhook_types.go b/v2/api/sql/v1/webhook/user_webhook_types.go index 412a0f09878..b2ebb6824fd 100644 --- a/v2/api/sql/v1/webhook/user_webhook_types.go +++ b/v2/api/sql/v1/webhook/user_webhook_types.go @@ -26,7 +26,7 @@ var _ webhook.CustomDefaulter = &User_Webhook{} func (webhook *User_Webhook) Default(ctx context.Context, obj runtime.Object) error { resource, ok := obj.(*v1.User) if !ok { - return fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/dbforpostgresql/v1/User, but got %T", obj) + return fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/sql/v1/User, but got %T", obj) } err := webhook.defaultImpl(ctx, resource) if err != nil { @@ -68,7 +68,7 @@ var _ webhook.CustomValidator = &User_Webhook{} func (webhook *User_Webhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { resource, ok := obj.(*v1.User) if !ok { - return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/dbforpostgresql/v1/User, but got %T", obj) + return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/sql/v1/User, but got %T", obj) } validations := webhook.createValidations() var temp any = webhook @@ -82,7 +82,7 @@ func (webhook *User_Webhook) ValidateCreate(ctx context.Context, obj runtime.Obj func (webhook *User_Webhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { resource, ok := obj.(*v1.User) if !ok { - return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/dbforpostgresql/v1/User, but got %T", obj) + return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/sql/v1/User, but got %T", obj) } validations := webhook.deleteValidations() var temp any = webhook @@ -96,11 +96,11 @@ func (webhook *User_Webhook) ValidateDelete(ctx context.Context, obj runtime.Obj func (webhook *User_Webhook) ValidateUpdate(ctx context.Context, oldObj runtime.Object, newObj runtime.Object) (admission.Warnings, error) { newResource, ok := newObj.(*v1.User) if !ok { - return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/dbforpostgresql/v1/User, but got %T", newObj) + return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/sql/v1/User, but got %T", newObj) } oldResource, ok := oldObj.(*v1.User) if !ok { - return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/dbforpostgresql/v1/User, but got %T", oldObj) + return nil, fmt.Errorf("expected github.com/Azure/azure-service-operator/v2/api/sql/v1/User, but got %T", oldObj) } validations := webhook.updateValidations() var temp any = webhook @@ -116,7 +116,9 @@ func (webhook *User_Webhook) ValidateUpdate(ctx context.Context, oldObj runtime. // createValidations validates the creation of the resource func (webhook *User_Webhook) createValidations() []func(ctx context.Context, obj *v1.User) (admission.Warnings, error) { - return nil + return []func(ctx context.Context, obj *v1.User) (admission.Warnings, error){ + webhook.validateIsLocalOrAAD, + } } // deleteValidations validates the deletion of the resource @@ -127,7 +129,12 @@ func (webhook *User_Webhook) deleteValidations() []func(ctx context.Context, obj // updateValidations validates the update of the resource func (webhook *User_Webhook) updateValidations() []func(ctx context.Context, oldObj *v1.User, newObj *v1.User) (admission.Warnings, error) { return []func(ctx context.Context, oldObj *v1.User, newObj *v1.User) (admission.Warnings, error){ + func(ctx context.Context, oldObj *v1.User, newObj *v1.User) (admission.Warnings, error) { + return webhook.validateIsLocalOrAAD(ctx, newObj) + }, + webhook.validateUserTypeNotChanged, webhook.validateWriteOncePropertiesNotChanged, + webhook.validateUserAADAliasNotChanged, } } @@ -177,3 +184,47 @@ func (webhook *User_Webhook) validateWriteOncePropertiesNotChanged(_ context.Con return nil, kerrors.NewAggregate(errs) } + +// validateIsLocalOrAAD validates that exactly one of LocalUser or AADUser is specified +func (webhook *User_Webhook) validateIsLocalOrAAD(_ context.Context, obj *v1.User) (admission.Warnings, error) { + if obj.Spec.LocalUser == nil && obj.Spec.AADUser == nil { + return nil, eris.Errorf("exactly one of spec.localUser or spec.aadUser must be set") + } + + if obj.Spec.LocalUser != nil && obj.Spec.AADUser != nil { + return nil, eris.Errorf("exactly one of spec.localUser or spec.aadUser must be set, not both") + } + + return nil, nil +} + +// validateUserTypeNotChanged prevents changing user type after creation +func (webhook *User_Webhook) validateUserTypeNotChanged(_ context.Context, oldObj *v1.User, newObj *v1.User) (admission.Warnings, error) { + // Prevent change from AAD -> Local + if oldObj.Spec.AADUser != nil && newObj.Spec.AADUser == nil { + return nil, eris.Errorf("cannot change from AAD user to local user") + } + + // Prevent change from Local -> AAD + if oldObj.Spec.LocalUser != nil && newObj.Spec.LocalUser == nil { + return nil, eris.Errorf("cannot change from local user to AAD user") + } + + return nil, nil +} + +// validateUserAADAliasNotChanged prevents changing AAD alias after creation +func (webhook *User_Webhook) validateUserAADAliasNotChanged(_ context.Context, oldObj *v1.User, newObj *v1.User) (admission.Warnings, error) { + if oldObj.Spec.AADUser == nil || newObj.Spec.AADUser == nil { + return nil, nil + } + + oldAlias := oldObj.Spec.AADUser.Alias + newAlias := newObj.Spec.AADUser.Alias + + if oldAlias != newAlias { + return nil, eris.Errorf("cannot change AAD user 'alias' from %q to %q", oldAlias, newAlias) + } + + return nil, nil +} From bf3af24786a6c6993ac94614e5b53974f1b8d32c Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:01 +0100 Subject: [PATCH 3/6] feat(sql): add AAD user connector for Azure SQL --- v2/internal/reconcilers/azuresql/aad_user.go | 143 +++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 v2/internal/reconcilers/azuresql/aad_user.go diff --git a/v2/internal/reconcilers/azuresql/aad_user.go b/v2/internal/reconcilers/azuresql/aad_user.go new file mode 100644 index 00000000000..963b6111621 --- /dev/null +++ b/v2/internal/reconcilers/azuresql/aad_user.go @@ -0,0 +1,143 @@ +/* +Copyright (c) Microsoft Corporation. +Licensed under the MIT license. +*/ + +package sql + +import ( + "context" + "database/sql" + + . "github.com/Azure/azure-service-operator/v2/internal/logging" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/go-logr/logr" + "github.com/rotisserie/eris" + + asosql "github.com/Azure/azure-service-operator/v2/api/sql/v1" + "github.com/Azure/azure-service-operator/v2/internal/identity" + "github.com/Azure/azure-service-operator/v2/internal/resolver" + azuresqlutil "github.com/Azure/azure-service-operator/v2/internal/util/azuresql" + "github.com/Azure/azure-service-operator/v2/pkg/genruntime/conditions" +) + +// Scope is the token scope for Azure SQL Database +const Scope = "https://database.windows.net/.default" + +type aadUser struct { + user *asosql.User + resourceResolver *resolver.Resolver + credentialProvider identity.CredentialProvider + log logr.Logger +} + +var _ Connector = &aadUser{} + +func (u *aadUser) CreateOrUpdate(ctx context.Context) error { + db, err := u.connectToDB(ctx) + if err != nil { + return err + } + defer db.Close() + + u.log.V(Status).Info("Creating/updating Azure SQL AAD user") + + username := u.username() + err = azuresqlutil.CreateOrUpdateAADUser(ctx, db, username) + if err != nil { + return eris.Wrap(err, "failed to create/update AAD user") + } + + // Ensure that the roles are set + err = azuresqlutil.ReconcileUserRoles(ctx, db, username, u.user.Spec.Roles) + if err != nil { + return eris.Wrap(err, "ensuring database roles") + } + + u.log.V(Status).Info("Successfully reconciled Azure SQL AAD user") + return nil +} + +func (u *aadUser) Delete(ctx context.Context) error { + db, err := u.connectToDB(ctx) + if err != nil { + return err + } + defer db.Close() + + username := u.username() + err = azuresqlutil.DropUser(ctx, db, username) + if err != nil { + return err + } + + return nil +} + +func (u *aadUser) Exists(ctx context.Context) (bool, error) { + db, err := u.connectToDB(ctx) + if err != nil { + return false, err + } + defer db.Close() + + username := u.username() + exists, err := azuresqlutil.DoesUserExist(ctx, db, username) + if err != nil { + return false, err + } + + return exists, nil +} + +func (u *aadUser) connectToDB(ctx context.Context) (*sql.DB, error) { + details, err := getOwnerDetails(ctx, u.resourceResolver, u.user) + if err != nil { + return nil, err + } + + if u.user.Spec.AADUser == nil { + return nil, eris.Errorf("AAD User missing $.spec.aadUser field") + } + + credential, err := u.credentialProvider.GetCredential(ctx, u.user) + if err != nil { + err = eris.Wrap(err, "failed to get credential for AAD authentication") + return nil, conditions.NewReadyConditionImpactingError(err, conditions.ConditionSeverityWarning, conditions.ReasonFailed) + } + + token, err := credential.TokenCredential().GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{Scope}}) + if err != nil { + err = eris.Wrap(err, "failed to get token from credential") + return nil, conditions.NewReadyConditionImpactingError(err, conditions.ConditionSeverityWarning, conditions.ReasonFailed) + } + u.log.V(Verbose).Info("Retrieved token for Azure SQL", "scope", Scope, "expires", token.ExpiresOn) + + // Connect to the DB using AAD token + db, err := azuresqlutil.ConnectToDBUsingAAD( + ctx, + details.fqdn, + details.database, + azuresqlutil.ServerPort, + func() (string, error) { return token.Token, nil }, + ) + if err != nil { + return nil, eris.Wrapf( + err, + "failed to connect to database using AAD. Server: %s, Database: %s, Port: %d", + details.fqdn, + details.database, + azuresqlutil.ServerPort) + } + + return db, nil +} + +func (u *aadUser) username() string { + if u.user.Spec.AADUser != nil && u.user.Spec.AADUser.Alias != "" { + return u.user.Spec.AADUser.Alias + } + + return u.user.Spec.AzureName +} From b1fb5264c9de1990831c92d55c2378d79b4a8597 Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:01 +0100 Subject: [PATCH 4/6] feat(sql): update reconciler to support AAD users --- .../reconcilers/azuresql/azuresql_user_reconciler.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/v2/internal/reconcilers/azuresql/azuresql_user_reconciler.go b/v2/internal/reconcilers/azuresql/azuresql_user_reconciler.go index 069789cd39d..0daf82e7fa3 100644 --- a/v2/internal/reconcilers/azuresql/azuresql_user_reconciler.go +++ b/v2/internal/reconcilers/azuresql/azuresql_user_reconciler.go @@ -181,7 +181,16 @@ func (r *AzureSQLUserReconciler) newDBConnector(log logr.Logger, user *asosql.Us }, nil } + if user.Spec.AADUser != nil { + return &aadUser{ + user: user, + resourceResolver: r.ResourceResolver, + credentialProvider: r.CredentialProvider, + log: log, + }, nil + } + // This is also enforced with a webhook - err := eris.Errorf("unknown user type, user must be LocalUser") + err := eris.Errorf("unknown user type, user must be LocalUser or AADUser") return nil, conditions.NewReadyConditionImpactingError(err, conditions.ConditionSeverityError, conditions.ReasonFailed) } From 5e456adce8290be28896317e4fb9e89c41d232b2 Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:01 +0100 Subject: [PATCH 5/6] test(sql): add integration tests for AAD users --- v2/test/azuresql_aad_test.go | 153 +++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 v2/test/azuresql_aad_test.go diff --git a/v2/test/azuresql_aad_test.go b/v2/test/azuresql_aad_test.go new file mode 100644 index 00000000000..1effdb49cb6 --- /dev/null +++ b/v2/test/azuresql_aad_test.go @@ -0,0 +1,153 @@ +/* +Copyright (c) Microsoft Corporation. +Licensed under the MIT license. +*/ + +package test + +import ( + "testing" + + . "github.com/onsi/gomega" + + _ "github.com/microsoft/go-mssqldb" + + managedidentity "github.com/Azure/azure-service-operator/v2/api/managedidentity/v1api20181130" + resources "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601" + azuresqlv1 "github.com/Azure/azure-service-operator/v2/api/sql/v1" + sql "github.com/Azure/azure-service-operator/v2/api/sql/v1api20211101" + "github.com/Azure/azure-service-operator/v2/internal/testcommon" + "github.com/Azure/azure-service-operator/v2/internal/util/to" +) + +func Test_AzureSQL_AADUser(t *testing.T) { + t.Parallel() + t.Skip("Cannot run in CI currently due to AAD tenant restrictions on CI subscription") + + tc := globalTestContext.ForTest(t) + tc.AzureRegion = to.Ptr("eastus") + + rg := tc.CreateTestResourceGroupAndWait() + + server := newAzureSQLServerWithAADAdmin(tc, rg) + database := newAzureSQLServerDatabase(tc, server) + firewallRule := newSQLServerOpenFirewallRule(tc, server) + + tc.CreateResourcesAndWait(server, database, firewallRule) + tc.Expect(server.Status.FullyQualifiedDomainName).ToNot(BeNil()) + + tc.RunSubtests( + testcommon.Subtest{ + Name: "AzureSQL AAD User CRUD", + Test: func(testContext *testcommon.KubePerTestContext) { + AzureSQL_AADUser_CRUD(testContext, server, database, rg) + }, + }, + testcommon.Subtest{ + Name: "AzureSQL AAD User With Alias", + Test: func(testContext *testcommon.KubePerTestContext) { + AzureSQL_AADUser_WithAlias(testContext, server, database, rg) + }, + }, + ) +} + +func AzureSQL_AADUser_CRUD( + tc *testcommon.KubePerTestContext, + server *sql.Server, + database *sql.ServersDatabase, + rg *resources.ResourceGroup, +) { + mi := &managedidentity.UserAssignedIdentity{ + ObjectMeta: tc.MakeObjectMeta("mi"), + Spec: managedidentity.UserAssignedIdentity_Spec{ + Location: tc.AzureRegion, + Owner: testcommon.AsOwner(rg), + }, + } + tc.CreateResourceAndWait(mi) + + user := &azuresqlv1.User{ + ObjectMeta: tc.MakeObjectMetaWithName(mi.Name), + Spec: azuresqlv1.UserSpec{ + Owner: testcommon.AsOwner(database), + Roles: []string{"db_datareader"}, + AADUser: &azuresqlv1.AADUserSpec{ + ServerAdminUsername: "yourAADAdminName", + }, + }, + } + tc.CreateResourceAndWait(user) + tc.Expect(user.Status.Conditions).ToNot(BeEmpty()) + + old := user.DeepCopy() + user.Spec.Roles = []string{"db_datareader", "db_datawriter"} + tc.PatchResourceAndWait(old, user) + + originalUser := user.DeepCopy() + + old = user.DeepCopy() + user.Spec.AADUser = nil + user.Spec.LocalUser = &azuresqlv1.LocalUserSpec{ServerAdminUsername: "someadmin"} + err := tc.PatchAndExpectError(old, user) + tc.Expect(err).To(HaveOccurred()) + tc.Expect(err.Error()).To(ContainSubstring("cannot change from AAD user to local user")) + + user = originalUser.DeepCopy() + tc.DeleteResourceAndWait(user) +} + +func AzureSQL_AADUser_WithAlias( + tc *testcommon.KubePerTestContext, + server *sql.Server, + database *sql.ServersDatabase, + rg *resources.ResourceGroup, +) { + alias := "myaliaseduser" + + mi := &managedidentity.UserAssignedIdentity{ + ObjectMeta: tc.MakeObjectMeta("mi-long-name"), + Spec: managedidentity.UserAssignedIdentity_Spec{ + Location: tc.AzureRegion, + Owner: testcommon.AsOwner(rg), + }, + } + tc.CreateResourceAndWait(mi) + + user := &azuresqlv1.User{ + ObjectMeta: tc.MakeObjectMetaWithName("aaduser-alias"), + Spec: azuresqlv1.UserSpec{ + AzureName: mi.Name, + Owner: testcommon.AsOwner(database), + Roles: []string{"db_datareader"}, + AADUser: &azuresqlv1.AADUserSpec{ + Alias: alias, + ServerAdminUsername: "yourAADAdminName", + }, + }, + } + tc.CreateResourceAndWait(user) + + originalUser := user.DeepCopy() + + old := user.DeepCopy() + user.Spec.AADUser.Alias = "adifferentalias" + err := tc.PatchAndExpectError(old, user) + tc.Expect(err).To(HaveOccurred()) + tc.Expect(err.Error()).To(ContainSubstring("cannot change AAD user 'alias'")) + + user = originalUser.DeepCopy() + tc.DeleteResourceAndWait(user) +} + +func newAzureSQLServerWithAADAdmin(tc *testcommon.KubePerTestContext, rg *resources.ResourceGroup) *sql.Server { + server := &sql.Server{ + ObjectMeta: tc.MakeObjectMeta("sqlserver"), + Spec: sql.Server_Spec{ + Location: tc.AzureRegion, + Owner: testcommon.AsOwner(rg), + Version: to.Ptr("12.0"), + }, + } + return server +} From 1219643c9e2f53328681876769a79fba92379e99 Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:01 +0100 Subject: [PATCH 6/6] test(sql): add webhook unit tests for User resource --- v2/api/sql/v1/webhook/user_webhook_test.go | 193 +++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 v2/api/sql/v1/webhook/user_webhook_test.go diff --git a/v2/api/sql/v1/webhook/user_webhook_test.go b/v2/api/sql/v1/webhook/user_webhook_test.go new file mode 100644 index 00000000000..0e71b67abbb --- /dev/null +++ b/v2/api/sql/v1/webhook/user_webhook_test.go @@ -0,0 +1,193 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +package webhook + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1 "github.com/Azure/azure-service-operator/v2/api/sql/v1" + "github.com/Azure/azure-service-operator/v2/pkg/genruntime" +) + +func Test_UserWebhook_ValidateIsLocalOrAAD(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + webhook := &User_Webhook{} + ctx := context.Background() + + tests := []struct { + name string + user *v1.User + wantErr string + }{ + { + name: "valid local user", + user: &v1.User{ + Spec: v1.UserSpec{ + LocalUser: &v1.LocalUserSpec{ + ServerAdminUsername: "admin", + ServerAdminPassword: &genruntime.SecretReference{Name: "secret", Key: "password"}, + Password: &genruntime.SecretReference{Name: "secret", Key: "userpass"}, + }, + }, + }, + wantErr: "", + }, + { + name: "valid AAD user", + user: &v1.User{ + Spec: v1.UserSpec{ + AADUser: &v1.AADUserSpec{ + ServerAdminUsername: "admin", + }, + }, + }, + wantErr: "", + }, + { + name: "neither specified", + user: &v1.User{Spec: v1.UserSpec{}}, + wantErr: "exactly one of spec.localUser or spec.aadUser must be set", + }, + { + name: "both specified", + user: &v1.User{ + Spec: v1.UserSpec{ + LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin"}, + AADUser: &v1.AADUserSpec{ServerAdminUsername: "admin"}, + }, + }, + wantErr: "exactly one of spec.localUser or spec.aadUser must be set", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := webhook.validateIsLocalOrAAD(ctx, tt.user) + if tt.wantErr == "" { + g.Expect(err).To(BeNil()) + } else { + g.Expect(err).ToNot(BeNil()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + } + }) + } +} + +func Test_UserWebhook_ValidateUserTypeNotChanged(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + webhook := &User_Webhook{} + ctx := context.Background() + + tests := []struct { + name string + oldUser *v1.User + newUser *v1.User + wantErr string + }{ + { + name: "local to local - allowed", + oldUser: &v1.User{Spec: v1.UserSpec{LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin"}}}, + newUser: &v1.User{Spec: v1.UserSpec{LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin2"}}}, + wantErr: "", + }, + { + name: "aad to aad - allowed", + oldUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{ServerAdminUsername: "admin"}}}, + newUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{ServerAdminUsername: "admin2"}}}, + wantErr: "", + }, + { + name: "local to aad - not allowed", + oldUser: &v1.User{Spec: v1.UserSpec{LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin"}}}, + newUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{ServerAdminUsername: "admin"}}}, + wantErr: "cannot change from local user to AAD user", + }, + { + name: "aad to local - not allowed", + oldUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{ServerAdminUsername: "admin"}}}, + newUser: &v1.User{Spec: v1.UserSpec{LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin"}}}, + wantErr: "cannot change from AAD user to local user", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := webhook.validateUserTypeNotChanged(ctx, tt.oldUser, tt.newUser) + if tt.wantErr == "" { + g.Expect(err).To(BeNil()) + } else { + g.Expect(err).ToNot(BeNil()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + } + }) + } +} + +func Test_UserWebhook_ValidateUserAADAliasNotChanged(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + webhook := &User_Webhook{} + ctx := context.Background() + + tests := []struct { + name string + oldUser *v1.User + newUser *v1.User + wantErr string + }{ + { + name: "alias unchanged - allowed", + oldUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{Alias: "myalias"}}}, + newUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{Alias: "myalias"}}}, + wantErr: "", + }, + { + name: "alias changed - not allowed", + oldUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{Alias: "oldalias"}}}, + newUser: &v1.User{Spec: v1.UserSpec{AADUser: &v1.AADUserSpec{Alias: "newalias"}}}, + wantErr: "cannot change AAD user 'alias'", + }, + { + name: "local user - skipped", + oldUser: &v1.User{Spec: v1.UserSpec{LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin"}}}, + newUser: &v1.User{Spec: v1.UserSpec{LocalUser: &v1.LocalUserSpec{ServerAdminUsername: "admin"}}}, + wantErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := webhook.validateUserAADAliasNotChanged(ctx, tt.oldUser, tt.newUser) + if tt.wantErr == "" { + g.Expect(err).To(BeNil()) + } else { + g.Expect(err).ToNot(BeNil()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + } + }) + } +} + +func Test_UserWebhook_ValidateCreate(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + webhook := &User_Webhook{} + ctx := context.Background() + + user := &v1.User{ + ObjectMeta: metav1.ObjectMeta{Name: "test-user", Namespace: "default"}, + Spec: v1.UserSpec{ + AzureName: "my-managed-identity", + AADUser: &v1.AADUserSpec{ServerAdminUsername: "admin"}, + }, + } + + _, err := webhook.ValidateCreate(ctx, user) + g.Expect(err).To(BeNil()) +}