From fd5565efc32c2f6fc9f6b9c76b5b6bb5f5bf9b02 Mon Sep 17 00:00:00 2001 From: Janani Vasudevan <49576785+jananivMS@users.noreply.github.com> Date: Wed, 4 Sep 2019 16:12:36 -0600 Subject: [PATCH] Changes to the controller to support allowing azure access to SQL server (#184) * feat: implement keyvault controller * Ace's KV changes with updates * Added an event for the final successful provisioning * Updated changes based on the PR comments * removing unwanted file * making resource group name the one in the keyvault yaml * need to handled unexpected error types...like validation.error (#111) * refactor tests (#90) * improve tests with parallel execution and rm sleep * fix the tests to run on kindcluster * Updates to KV controller from Ace (#80) (#112) * feat: implement keyvault controller * Ace's KV changes with updates * Added an event for the final successful provisioning * Updated changes based on the PR comments * removing unwanted file * making resource group name the one in the keyvault yaml Co-authored-by: Ace Eldeib * Test update (#115) * this needs to exist in the reconciler in order to use controllerutil createorupdate * Feat/add consumer group kind (#117) * add consumer group kind * update tests for consumer group * fix isbeingdeleted * Updates to README - steps for onboarding (#114) * cluster additions * updated docs * Update azure-pipelines.yaml (#119) * Update azure-pipelines.yaml * fix tests (#140) * revert back // +kubebuilder:subresource:status changes - fix broken tests * Devcontainer to Help Onboard New People (#142) * add dev conatiner - wip * DevContainer up and running. * Removed `sleep 80` and replaced with `kubectl wait`. * Run `make set-kindcluster` from docker-compose. * Set timeout on wait. * Added `install-test-dependency` to makefile and dockerfile. * Update README - Create SP with contribution rights. * Updated README with details on using devcontainer. * Stuff that wanted me to commit. * Reverted changes made to `docker-build` in Makefile. * pass future where possible instead of bool (#121) * first commit on Amanda's branch * first * before properties * test not tested * test works * unit tests work, needs firewall rules * addresses feedback * erin's feedback * janani's change, pass future * async works much better now * janani feedback * screwed up interface prototype * randomize the resources names used in tests (#152) * Ability to Set SecretName When Creating Event Hub (#151) * Updated eventhub_types - Added `secretName`. * Added `secretName` to sample manifest. * Set secret name to `secretName` if set, otherwise use eventhub name. * Updated Makefile to update Azure Operator. Also added the ability to rebuild image without cache. * Updated README on how to update the Azure Operator. * Updated CRD with SecretName description. * Added tests to ensure `SecretName` was being used if present. * Fix test. * Pr 22 merge (#158) * kubebuilder init --domain azure --license none * kubebuilder create api --group service --version v1alpha1 --kind Storage * kubebuilder create api --group service --version v1alpha1 --kind CosmosDB * Add MIT License * Initial codes to support Azure Storage Account * Add development docs * Remove the storage account name from the spec * Sync additional resources for Azure storage account 1. Create a secret based on storage account credentials 2. Add the global config * Upgrade kubebuilder to 2.0.0-beta.0 and controller-runtime to v0.2.0-beta.4 * Copy pkg in Dockerfile * Update controller-gen and make manifests * Add prefix "Storage" for storage_types * feature: add redis cache service * Ignore the NotFound error when deleting resources * Requeue the request if the deployment is not complete * feature: add cosmosdb service * Refine the logic of updating additional resources and output * Deploy operator on a remote cluster * add a sample app deployment yaml * Generate assets for the templates * Requeue after 30 seconds to avoid too many requests Ignore the NotFound error when deleting cosmosdb * Fix a bug of missing capacity of rediscache template * fix: judge whether resources need to be updated With adding generation in status, we can judge whether resources need to be updated. Co-authored-by: Bin Xia * Add docs to run the demo * Update manager-role to operate secrets Workaround: the rule should be appended. But I don't know how for now. The workaround is to copy config/rbac/role.yaml and add the new rule. Should be fixed in future. * fix(Makefile): rename the target from "generate" to "generate-template" to avoid conflict * Refactoring data focused operators. Storage currently working though it needs cleanup * Added deepcopy generated code * CosmosDB deploy working * Detailing current implementation of CosmosDB Create parameters * Removing TestTags * Redis cache now deploys * Cleaned up code and removed references to v1alpha1 * Updating controllers logging calls Co-authored-by: Chris Risner Co-authored-by: Bin Xia * Pr 22 merge (#158) * kubebuilder init --domain azure --license none * kubebuilder create api --group service --version v1alpha1 --kind Storage * kubebuilder create api --group service --version v1alpha1 --kind CosmosDB * Add MIT License * Initial codes to support Azure Storage Account * Add development docs * Remove the storage account name from the spec * Sync additional resources for Azure storage account 1. Create a secret based on storage account credentials 2. Add the global config * Upgrade kubebuilder to 2.0.0-beta.0 and controller-runtime to v0.2.0-beta.4 * Copy pkg in Dockerfile * Update controller-gen and make manifests * Add prefix "Storage" for storage_types * feature: add redis cache service * Ignore the NotFound error when deleting resources * Requeue the request if the deployment is not complete * feature: add cosmosdb service * Refine the logic of updating additional resources and output * Deploy operator on a remote cluster * add a sample app deployment yaml * Generate assets for the templates * Requeue after 30 seconds to avoid too many requests Ignore the NotFound error when deleting cosmosdb * Fix a bug of missing capacity of rediscache template * fix: judge whether resources need to be updated With adding generation in status, we can judge whether resources need to be updated. Co-authored-by: Bin Xia * Add docs to run the demo * Update manager-role to operate secrets Workaround: the rule should be appended. But I don't know how for now. The workaround is to copy config/rbac/role.yaml and add the new rule. Should be fixed in future. * fix(Makefile): rename the target from "generate" to "generate-template" to avoid conflict * Refactoring data focused operators. Storage currently working though it needs cleanup * Added deepcopy generated code * CosmosDB deploy working * Detailing current implementation of CosmosDB Create parameters * Removing TestTags * Redis cache now deploys * Cleaned up code and removed references to v1alpha1 * Updating controllers logging calls Co-authored-by: Chris Risner Co-authored-by: Bin Xia * Pr 22 merge (#158) (#165) * kubebuilder init --domain azure --license none * kubebuilder create api --group service --version v1alpha1 --kind Storage * kubebuilder create api --group service --version v1alpha1 --kind CosmosDB * Add MIT License * Initial codes to support Azure Storage Account * Add development docs * Remove the storage account name from the spec * Sync additional resources for Azure storage account 1. Create a secret based on storage account credentials 2. Add the global config * Upgrade kubebuilder to 2.0.0-beta.0 and controller-runtime to v0.2.0-beta.4 * Copy pkg in Dockerfile * Update controller-gen and make manifests * Add prefix "Storage" for storage_types * feature: add redis cache service * Ignore the NotFound error when deleting resources * Requeue the request if the deployment is not complete * feature: add cosmosdb service * Refine the logic of updating additional resources and output * Deploy operator on a remote cluster * add a sample app deployment yaml * Generate assets for the templates * Requeue after 30 seconds to avoid too many requests Ignore the NotFound error when deleting cosmosdb * Fix a bug of missing capacity of rediscache template * fix: judge whether resources need to be updated With adding generation in status, we can judge whether resources need to be updated. Co-authored-by: Bin Xia * Add docs to run the demo * Update manager-role to operate secrets Workaround: the rule should be appended. But I don't know how for now. The workaround is to copy config/rbac/role.yaml and add the new rule. Should be fixed in future. * fix(Makefile): rename the target from "generate" to "generate-template" to avoid conflict * Refactoring data focused operators. Storage currently working though it needs cleanup * Added deepcopy generated code * CosmosDB deploy working * Detailing current implementation of CosmosDB Create parameters * Removing TestTags * Redis cache now deploys * Cleaned up code and removed references to v1alpha1 * Updating controllers logging calls Co-authored-by: Chris Risner Co-authored-by: Bin Xia * Capture EventHub to Azure Blob Storage Container (#146) * added eventhub with and without capture * create, delete and get properties for storage manager * capture eventhub tests * added storage tests to make tests * configured location to default set by environment variable * synchronised test setup and teardown * incorporated storages module * fixed setup and teardown of storage tests * fixed storage container tests * Camelcase EventHub (#176) * Removed ports from docker-compose. * Updated CRD - camelcase over lowercase. * Updated example manifests. * Role thing. * Camelcase new changes to EventHub types. * Camelcase example. * Removed old file. * Fixing issues #173 and #174 (#175) * Updated controllers to use `azure.microsoft.com` over `service.azure`. * Updated webhooks to point to `azure.microsoft.com`. * Updated caninject to point to `azure.microsoft.com`. * Regenerated role.yaml. * Point kustomization.yaml in CRD to right base CRDs. * Updated demo. * Role update. * Update group from service to azure in PROJECT. * Increased Partition Count Minimum in EventHub to 2 (#178) * Increase minimum partition count to 2. * Updated the CRD. * Updated eventhub example. * Changed resource group example. * Increased test partition count to 2. * Updated tests. * Changes to wire Sql firewall in the controller * Addressed PR comments --- api/v1/sqlserver_types.go | 7 +- .../bases/azure.microsoft.com_sqlservers.yaml | 2 + config/rbac/role.yaml | 126 +++++++++--------- config/samples/azure_v1_sqlserver.yaml | 1 + controllers/sqlserver_controller.go | 12 +- .../sqlclient/endtoend_test.go | 1 - .../sqlclient/sqlproperties.go | 3 - 7 files changed, 79 insertions(+), 73 deletions(-) diff --git a/api/v1/sqlserver_types.go b/api/v1/sqlserver_types.go index 66d521a87e0..1baf4f89978 100644 --- a/api/v1/sqlserver_types.go +++ b/api/v1/sqlserver_types.go @@ -26,9 +26,10 @@ import ( type SqlServerSpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file - Location string `json:"location"` - ResourceGroup string `json:"resourcegroup,omitempty"` - AdminUser string `json:"adminuser,omitempty"` + Location string `json:"location"` + ResourceGroup string `json:"resourcegroup,omitempty"` + AdminUser string `json:"adminuser,omitempty"` + AllowAzureServiceAccess bool `json:"allowazureserviceaccess,omitempty"` } // SqlServerStatus defines the observed state of SqlServer diff --git a/config/crd/bases/azure.microsoft.com_sqlservers.yaml b/config/crd/bases/azure.microsoft.com_sqlservers.yaml index 5cfda095898..78f7bcfb9ff 100644 --- a/config/crd/bases/azure.microsoft.com_sqlservers.yaml +++ b/config/crd/bases/azure.microsoft.com_sqlservers.yaml @@ -403,6 +403,8 @@ spec: properties: adminuser: type: string + allowazureserviceaccess: + type: boolean location: description: 'INSERT ADDITIONAL SPEC FIELDS - desired state of cluster Important: Run "make" to regenerate code after modifying this file' diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index df285ac3214..ac6357acf27 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -9,19 +9,15 @@ rules: - apiGroups: - azure.microsoft.com resources: - - eventhubs + - resourcegroups/status verbs: - - create - - delete - get - - list - patch - update - - watch - apiGroups: - - azure.microsoft.com + - apps resources: - - sqlservers + - deployments verbs: - create - delete @@ -33,7 +29,7 @@ rules: - apiGroups: - azure.microsoft.com resources: - - storages/status + - consumergroups/status verbs: - get - patch @@ -41,15 +37,19 @@ rules: - apiGroups: - azure.microsoft.com resources: - - sqlservers/status + - rediscaches verbs: + - create + - delete - get + - list - patch - update + - watch - apiGroups: - azure.microsoft.com resources: - - cosmosdbs + - keyvaults verbs: - create - delete @@ -61,15 +61,7 @@ rules: - apiGroups: - azure.microsoft.com resources: - - resourcegroups/status - verbs: - - get - - patch - - update -- apiGroups: - - "" - resources: - - secrets + - sqlservers verbs: - create - delete @@ -79,9 +71,9 @@ rules: - update - watch - apiGroups: - - apps + - azure.microsoft.com resources: - - deployments + - eventhubs verbs: - create - delete @@ -91,25 +83,33 @@ rules: - update - watch - apiGroups: - - apps + - azure.microsoft.com resources: - - deployments/status + - sqldatabases verbs: + - create + - delete - get + - list - patch - update + - watch - apiGroups: - azure.microsoft.com resources: - - cosmosdbs/status + - sqlfirewallrules verbs: + - create + - delete - get + - list - patch - update + - watch - apiGroups: - azure.microsoft.com resources: - - eventhubnamespaces/status + - storages/status verbs: - get - patch @@ -117,7 +117,7 @@ rules: - apiGroups: - azure.microsoft.com resources: - - keyvaults + - consumergroups verbs: - create - delete @@ -129,35 +129,31 @@ rules: - apiGroups: - azure.microsoft.com resources: - - rediscaches/status + - cosmosdbs verbs: + - create + - delete - get + - list - patch - update + - watch - apiGroups: - azure.microsoft.com resources: - - resourcegroups + - eventhubs/status verbs: - - create - - delete - get - - list - patch - update - - watch - apiGroups: - azure.microsoft.com resources: - - sqldatabases + - sqlfirewallrules/status verbs: - - create - - delete - get - - list - patch - update - - watch - apiGroups: - azure.microsoft.com resources: @@ -169,7 +165,7 @@ rules: - apiGroups: - azure.microsoft.com resources: - - storages + - eventhubnamespaces verbs: - create - delete @@ -181,61 +177,53 @@ rules: - apiGroups: - azure.microsoft.com resources: - - consumergroups + - eventhubnamespaces/status verbs: - - create - - delete - get - - list - patch - update - - watch - apiGroups: - azure.microsoft.com resources: - - eventhubnamespaces + - cosmosdbs/status verbs: - - create - - delete - get - - list - patch - update - - watch - apiGroups: - azure.microsoft.com resources: - - sqlfirewallrules + - sqlservers/status verbs: - - create - - delete - get - - list - patch - update - - watch - apiGroups: - azure.microsoft.com resources: - - sqlfirewallrules/status + - storages verbs: + - create + - delete - get + - list - patch - update + - watch - apiGroups: - - azure.microsoft.com + - "" resources: - events verbs: - create - - patch + - watch - apiGroups: - - "" + - azure.microsoft.com resources: - events verbs: - create - - watch + - patch - apiGroups: - azure.microsoft.com resources: @@ -247,7 +235,15 @@ rules: - apiGroups: - azure.microsoft.com resources: - - rediscaches + - rediscaches/status + verbs: + - get + - patch + - update +- apiGroups: + - azure.microsoft.com + resources: + - resourcegroups verbs: - create - delete @@ -257,17 +253,21 @@ rules: - update - watch - apiGroups: - - azure.microsoft.com + - "" resources: - - consumergroups/status + - secrets verbs: + - create + - delete - get + - list - patch - update + - watch - apiGroups: - - azure.microsoft.com + - apps resources: - - eventhubs/status + - deployments/status verbs: - get - patch diff --git a/config/samples/azure_v1_sqlserver.yaml b/config/samples/azure_v1_sqlserver.yaml index c05db4567ae..9da047d5282 100644 --- a/config/samples/azure_v1_sqlserver.yaml +++ b/config/samples/azure_v1_sqlserver.yaml @@ -5,3 +5,4 @@ metadata: spec: location: "westus" resourcegroup: "resourcegroup-sample-1907" + allowazureserviceaccess: true diff --git a/controllers/sqlserver_controller.go b/controllers/sqlserver_controller.go index fc22fc10d4c..b86245fcd50 100644 --- a/controllers/sqlserver_controller.go +++ b/controllers/sqlserver_controller.go @@ -109,7 +109,6 @@ func (r *SqlServerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } r.Recorder.Event(&instance, "Normal", "Provisioned", "sqlserver "+instance.ObjectMeta.Name+" provisioned ") - return ctrl.Result{}, nil } @@ -183,6 +182,15 @@ func (r *SqlServerReconciler) verifyExternal(instance *azurev1.SqlServer) error r.Recorder.Event(instance, "Normal", "Checking", fmt.Sprintf("instance in %s state", instance.Status.State)) if instance.Status.State == "Ready" { + + if instance.Spec.AllowAzureServiceAccess == true { + // Add firewall rule to allow azure service access + _, err := sdkClient.CreateOrUpdateSQLFirewallRule("AllowAzureAccess", "0.0.0.0", "0.0.0.0") + if err != nil { + r.Recorder.Event(instance, "Warning", "Failed", "Unable to add firewall rule to SQL server") + return errhelp.NewAzureError(err) + } + } instance.Status.Provisioned = true instance.Status.Provisioning = false } @@ -193,8 +201,6 @@ func (r *SqlServerReconciler) verifyExternal(instance *azurev1.SqlServer) error return updateerr } - // r.Recorder.Event(instance, "Normal", "Provisioned", "created or updated entity") - return errhelp.NewAzureError(err) } diff --git a/pkg/resourcemanager/sqlclient/endtoend_test.go b/pkg/resourcemanager/sqlclient/endtoend_test.go index 119de4a2812..f85d7854b42 100644 --- a/pkg/resourcemanager/sqlclient/endtoend_test.go +++ b/pkg/resourcemanager/sqlclient/endtoend_test.go @@ -45,7 +45,6 @@ func TestCreateOrUpdateSQLServer(t *testing.T) { sqlServerProperties := SQLServerProperties{ AdministratorLogin: to.StringPtr("Moss"), AdministratorLoginPassword: to.StringPtr("TheITCrowd_{01}!"), - AllowAzureServicesAccess: true, } // wait for server to be created, then only proceed once activated diff --git a/pkg/resourcemanager/sqlclient/sqlproperties.go b/pkg/resourcemanager/sqlclient/sqlproperties.go index 23f1116d3cc..7d3131d7685 100644 --- a/pkg/resourcemanager/sqlclient/sqlproperties.go +++ b/pkg/resourcemanager/sqlclient/sqlproperties.go @@ -53,9 +53,6 @@ type SQLServerProperties struct { // AdministratorLoginPassword - The administrator login password (required for server creation). AdministratorLoginPassword *string - - // AllowAzureServicesAccess - allow Azure services and resources to access this server - AllowAzureServicesAccess bool } // SQLDatabaseProperties contains values needed for adding / updating SQL servers,