Updated firewallrule to work like DB, other fixes... (#285)

* first push

* works

* janani feedback

* updated sample yaml

* added comment to YAML

Makefile

@@ -31,18 +31,6 @@ api-test: generate fmt vet manifests
 	TEST_USE_EXISTING_CLUSTER=false go test -v -coverprofile=coverage.txt -covermode count ./api/...  2>&1 | tee testlogs.txt
 	go-junit-report < testlogs.txt  > report.xml
 	go tool cover -html=coverage.txt -o cover.html
-
-# Generate test certs for development
-generate-test-certs:
-	echo "[req]" > config.txt
-	echo "distinguished_name = req_distinguished_name" >> config.txt
-	echo "[req_distinguished_name]" >> config.txt
-	echo "[SAN]" >> config.txt
-	echo "subjectAltName=DNS:azureoperator-webhook-service.azureoperator-system.svc.cluster.local" >> config.txt
-	openssl req -x509 -days 730 -out tls.crt -keyout tls.key -newkey rsa:4096 -subj "/CN=azureoperator-webhook-service.azureoperator-system" -config config.txt -nodes
-	rm -rf /tmp/k8s-webhook-server
-	mkdir -p /tmp/k8s-webhook-server/serving-certs
-	mv tls.* /tmp/k8s-webhook-server/serving-certs/
 
 # Run tests
 test: generate fmt vet manifests

api/v1/sqlfirewallrule_types.go

@@ -26,6 +26,7 @@ import (
 type SqlFirewallRuleSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
+	ResourceGroup  string `json:"resourcegroup,omitempty"`
 	Server         string `json:"server"`
 	StartIPAddress string `json:"startipaddress,omitempty"`
 	EndIPAddress   string `json:"endipaddress,omitempty"`

api/v1/sqlserver_types.go

@@ -26,9 +26,8 @@ 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"`
-	AllowAzureServiceAccess bool   `json:"allowazureserviceaccess,omitempty"`
+	Location      string `json:"location"`
+	ResourceGroup string `json:"resourcegroup,omitempty"`
 }
 
 // SqlServerStatus defines the observed state of SqlServer

api/v1/sqlserver_types_test.go

@@ -59,9 +59,8 @@ var _ = Describe("SqlServer", func() {
 					Namespace: "default",
 				},
 				Spec: SqlServerSpec{
-					Location:                "westus",
-					ResourceGroup:           "foo-resourcegroup",
-					AllowAzureServiceAccess: true,
+					Location:      "westus",
+					ResourceGroup: "foo-resourcegroup",
 				}}
 
 			By("creating an API obj")

config/samples/azure_v1_consumergroup.yaml

@@ -4,6 +4,6 @@ metadata:
   name: consumergroup-sample-1
 spec:
   # Add fields here
-  resourcegroup: "resourcegroup-sample-1"
+  resourcegroup: resourcegroup-azure-operators
   namespace: "eventhubnamespace-sample-1"
   eventHub: "eventhub-sample-2"

config/samples/azure_v1_cosmosdb.yaml

@@ -5,6 +5,6 @@ metadata:
 spec:
   kind: GlobalDocumentDB
   location: westus
-  resourceGroup: resourcegroup-sample-1908
+  resourceGroup: resourcegroup-azure-operators
   properties:
     databaseAccountOfferType: Standard

config/samples/azure_v1_eventhub.yaml

@@ -4,7 +4,7 @@ metadata:
   name: eventhub-sample-2307-09
 spec:
   location: "westus"
-  resourceGroup: "resourcegroup-sample-1907"
+  resourceGroup: resourcegroup-azure-operators
   namespace: "eventhubnamespace-sample-6"
   properties:
     messageRetentionInDays: 7

config/samples/azure_v1_eventhub_capture.yaml

@@ -4,7 +4,7 @@ metadata:
   name: eventhub-sample-2307-10-capture
 spec:
   location: "westus"
-  resourceGroup: "resourcegroup-sample-1907"
+  resourceGroup: resourcegroup-azure-operators
   namespace: "eventhubnamespace-sample-6"
   properties:
     messageRetentionInDays: 7

config/samples/azure_v1_eventhubnamespace.yaml

@@ -4,7 +4,7 @@ metadata:
   name: eventhubnamespace-sample-6
 spec:    
     location: "westus"
-    resourceGroup: "resourcegroup-sample-1907"
+    resourceGroup: resourcegroup-azure-operators
     sku:
       name: "Standard"
       tier: "Standard"

config/samples/azure_v1_keyvault.yaml

@@ -3,5 +3,5 @@ kind: KeyVault
 metadata:
   name: keyvaultsample123
 spec:
-  resourceGroup: resourcegroup-sample-6
+  resourceGroup: resourcegroup-azure-operators
   location: westus

config/samples/azure_v1_rediscache.yaml

@@ -4,7 +4,7 @@ metadata:
   name: rediscache-sample1908xyzkj
 spec:
   location: westus
-  resourceGroup: resourcegroup-sample-1908
+  resourceGroup: resourcegroup-azure-operators
   properties:
     sku:
       name: Basic

config/samples/azure_v1_resourcegroup.yaml

@@ -1,6 +1,6 @@
 apiVersion: azure.microsoft.com/v1
 kind: ResourceGroup
 metadata:
-  name: resourcegroup-sample-1907
+  name: resourcegroup-azure-operators
 spec:
   location: "westus"

config/samples/azure_v1_sqldatabase.yaml

@@ -4,6 +4,6 @@ metadata:
   name:  sqldatabase-sample
 spec:
   location: westus
-  resourcegroup: resourcegroup-sample-1907
-  edition: 0
-  server:  sqlserver-sample
+  resourcegroup: resourcegroup-azure-operators
+  server:  sqlserver-sample-777
+  edition: 0

config/samples/azure_v1_sqlfirewallrule.yaml

@@ -1,8 +1,11 @@
 apiVersion: azure.microsoft.com/v1
 kind: SqlFirewallRule
 metadata:
-  name: sqlfirewallrule-sample
+  name: sqlf-allowazuresvcaccess
 spec:
-  server:  sqlserver-sample
-  startipaddress: 1.1.1.1
-  endipaddress: 6.6.6.6
+  resourcegroup: resourcegroup-azure-operators
+  server:  sqlserver-sample-777
+
+  # this IP range enables Azure Service access
+  startipaddress: 0.0.0.0
+  endipaddress: 0.0.0.0

config/samples/azure_v1_sqlserver.yaml

@@ -1,8 +1,7 @@
 apiVersion: azure.microsoft.com/v1
 kind: SqlServer
 metadata:
-  name: sqlserver-sample
+  name: sqlserver-sample-777
 spec:
   location: "westus"
-  resourcegroup: "resourcegroup-sample-1907"
-  allowazureserviceaccess: true
+  resourcegroup: resourcegroup-azure-operators

config/samples/azure_v1_storage.yaml

@@ -4,7 +4,7 @@ metadata:
   name: storagesample1908ayzkj
 spec:
   location: westus
-  resourceGroup: resourcegroup-sample-1908
+  resourceGroup: resourcegroup-azure-operators
   sku:
     name: Standard_RAGRS
   kind: StorageV2

controllers/sqldatabase_controller.go

@@ -120,11 +120,10 @@ func (r *SqlDatabaseReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *SqlDatabaseReconciler) reconcileExternal(instance *azurev1.SqlDatabase) error {
 	ctx := context.Background()
 	location := instance.Spec.Location
-	server := instance.Spec.Server
 	groupName := instance.Spec.ResourceGroup
-	dbEdition := instance.Spec.Edition
-
+	server := instance.Spec.Server
 	dbName := instance.ObjectMeta.Name
+	dbEdition := instance.Spec.Edition
 
 	sdkClient := sql.GoSDKClient{
 		Ctx:               ctx,
@@ -146,7 +145,6 @@ func (r *SqlDatabaseReconciler) reconcileExternal(instance *azurev1.SqlDatabase)
 	var ownerInstance azurev1.SqlServer
 	sqlServerNamespacedName := types.NamespacedName{Name: server, Namespace: instance.Namespace}
 	err := r.Get(ctx, sqlServerNamespacedName, &ownerInstance)
-
 	if err != nil {
 		//log error and kill it, as the parent might not exist in the cluster. It could have been created elsewhere or through the portal directly
 		r.Recorder.Event(instance, "Warning", "Failed", "Unable to get owner instance of SqlServer")
@@ -194,10 +192,10 @@ func (r *SqlDatabaseReconciler) reconcileExternal(instance *azurev1.SqlDatabase)
 
 func (r *SqlDatabaseReconciler) deleteExternal(instance *azurev1.SqlDatabase) error {
 	ctx := context.Background()
-	dbname := instance.ObjectMeta.Name
+	location := instance.Spec.Location
 	groupName := instance.Spec.ResourceGroup
 	server := instance.Spec.Server
-	location := instance.Spec.Location
+	dbName := instance.ObjectMeta.Name
 
 	// create the Go SDK client with relevant info
 	sdk := sql.GoSDKClient{
@@ -207,8 +205,8 @@ func (r *SqlDatabaseReconciler) deleteExternal(instance *azurev1.SqlDatabase) er
 		Location:          location,
 	}
 
-	r.Log.Info(fmt.Sprintf("deleting external resource: group/%s/server/%s/database/%s"+groupName, server, dbname))
-	_, err := sdk.DeleteDB(dbname)
+	r.Log.Info(fmt.Sprintf("deleting external resource: group/%s/server/%s/database/%s"+groupName, server, dbName))
+	_, err := sdk.DeleteDB(dbName)
 	if err != nil {
 		if errhelp.IsStatusCode204(err) {
 			r.Recorder.Event(instance, "Warning", "DoesNotExist", "Resource to delete does not exist")
@@ -218,7 +216,7 @@ func (r *SqlDatabaseReconciler) deleteExternal(instance *azurev1.SqlDatabase) er
 		r.Recorder.Event(instance, "Warning", "Failed", "Couldn't delete resouce in azure")
 		return err
 	}
-	r.Recorder.Event(instance, "Normal", "Deleted", dbname+" deleted")
+	r.Recorder.Event(instance, "Normal", "Deleted", dbName+" deleted")
 	return nil
 }
 

controllers/sqlfirewallrule_controller.go

@@ -118,44 +118,42 @@ func (r *SqlFirewallRuleReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 func (r *SqlFirewallRuleReconciler) reconcileExternal(instance *azurev1.SqlFirewallRule) error {
 	ctx := context.Background()
-	ruleName := instance.ObjectMeta.Name
+	groupName := instance.Spec.ResourceGroup
 	server := instance.Spec.Server
+	ruleName := instance.ObjectMeta.Name
 	startIP := instance.Spec.StartIPAddress
 	endIP := instance.Spec.EndIPAddress
 
+	sdkClient := sql.GoSDKClient{
+		Ctx:               ctx,
+		ResourceGroupName: groupName,
+		ServerName:        server,
+	}
+
 	r.Log.Info("Calling createorupdate SQL firewall rule")
 
-	// get owner instance of SqlServer
+	//get owner instance of SqlServer
 	r.Recorder.Event(instance, "Normal", "UpdatingOwner", "Updating owner SqlServer instance")
 	var ownerInstance azurev1.SqlServer
 	sqlServerNamespacedName := types.NamespacedName{Name: server, Namespace: instance.Namespace}
 	err := r.Get(ctx, sqlServerNamespacedName, &ownerInstance)
 	if err != nil {
 		//log error and kill it, as the parent might not exist in the cluster. It could have been created elsewhere or through the portal directly
-		r.Recorder.Event(instance, "Warning", "Failed", "Unable to get owner instance of SqlServer when setting firewall parent")
-		return err
-	}
-
-	// set owner instance to the SqlServer instance
-	r.Recorder.Event(instance, "Normal", "OwnerAssign", "Got owner instance of Sql Server and assigning controller reference now")
-	err = controllerutil.SetControllerReference(&ownerInstance, instance, r.Scheme)
-	if err != nil {
-		r.Recorder.Event(instance, "Warning", "Failed", "Unable to set controller reference to SqlServer")
-		return err
+		r.Recorder.Event(instance, "Warning", "Failed", "Unable to get owner instance of SqlServer")
+	} else {
+		r.Recorder.Event(instance, "Normal", "OwnerAssign", "Got owner instance of Sql Server and assigning controller reference now")
+		innerErr := controllerutil.SetControllerReference(&ownerInstance, instance, r.Scheme)
+		if innerErr != nil {
+			r.Recorder.Event(instance, "Warning", "Failed", "Unable to set controller reference to SqlServer")
+		}
+		r.Recorder.Event(instance, "Normal", "OwnerAssign", "Owner instance assigned successfully")
 	}
-	r.Recorder.Event(instance, "Normal", "OwnerAssign", "Owner instance assigned successfully")
 
 	// write information back to instance
 	if err := r.Update(ctx, instance); err != nil {
 		r.Recorder.Event(instance, "Warning", "Failed", "Unable to update instance")
 	}
 
-	// actually add the firewall rule
-	sdkClient := sql.GoSDKClient{
-		Ctx:               ctx,
-		ResourceGroupName: ownerInstance.Spec.ResourceGroup,
-		ServerName:        server,
-	}
 	_, err = sdkClient.CreateOrUpdateSQLFirewallRule(ruleName, startIP, endIP)
 	if err != nil {
 		if errhelp.IsAsynchronousOperationNotComplete(err) || errhelp.IsGroupNotFound(err) {
@@ -186,19 +184,9 @@ func (r *SqlFirewallRuleReconciler) reconcileExternal(instance *azurev1.SqlFirew
 
 func (r *SqlFirewallRuleReconciler) deleteExternal(instance *azurev1.SqlFirewallRule) error {
 	ctx := context.Background()
-	ruleName := instance.ObjectMeta.Name
+	groupName := instance.Spec.ResourceGroup
 	server := instance.Spec.Server
-
-	//get owner instance of SqlServer
-	var ownerInstance azurev1.SqlServer
-	sqlServerNamespacedName := types.NamespacedName{Name: server, Namespace: instance.Namespace}
-	err := r.Get(ctx, sqlServerNamespacedName, &ownerInstance)
-	if err != nil {
-		//log error and kill it, as the parent might not exist in the cluster. It could have been created elsewhere or through the portal directly
-		r.Recorder.Event(instance, "Warning", "DoesNotExist", "Unable to get owner instance of SqlServer when removing firewall rule (OK)")
-		return nil
-	}
-	groupName := ownerInstance.Spec.ResourceGroup
+	ruleName := instance.ObjectMeta.Name
 
 	// create the Go SDK client with relevant info
 	sdk := sql.GoSDKClient{
@@ -208,7 +196,7 @@ func (r *SqlFirewallRuleReconciler) deleteExternal(instance *azurev1.SqlFirewall
 	}
 
 	r.Log.Info(fmt.Sprintf("deleting external resource: group/%s/server/%s/firewallrule/%s"+groupName, server, ruleName))
-	err = sdk.DeleteSQLFirewallRule(ruleName)
+	err := sdk.DeleteSQLFirewallRule(ruleName)
 	if err != nil {
 		if errhelp.IsStatusCode204(err) {
 			r.Recorder.Event(instance, "Warning", "DoesNotExist", "Resource to delete does not exist")

controllers/sqlserver_controller.go

@@ -72,17 +72,6 @@ func (r *SqlServerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	location := instance.Spec.Location
-	name := instance.ObjectMeta.Name
-	groupName := instance.Spec.ResourceGroup
-
-	sdkClient := sql.GoSDKClient{
-		Ctx:               ctx,
-		ResourceGroupName: groupName,
-		ServerName:        name,
-		Location:          location,
-	}
-
 	if helpers.IsBeingDeleted(&instance) {
 		if helpers.HasFinalizer(&instance, SQLServerFinalizerName) {
 			if err := r.deleteExternal(&instance); err != nil {
@@ -115,16 +104,27 @@ func (r *SqlServerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		}
 	}
 
-	availableResp, err := sdkClient.CheckNameAvailability()
-	if err != nil {
-		log.Info("error validating name")
-		return ctrl.Result{}, err
-	}
-	if !availableResp.Available {
-		log.Info("Servername is invalid or not available")
-		r.Recorder.Event(&instance, "Warning", "Failed", "Servername is invalid")
-		return ctrl.Result{Requeue: false}, fmt.Errorf("Servername invalid %s", availableResp.Name)
-	}
+	/*
+		location := instance.Spec.Location
+		name := instance.ObjectMeta.Name
+		groupName := instance.Spec.ResourceGroup
+		sdkClient := sql.GoSDKClient{
+			Ctx:               ctx,
+			ResourceGroupName: groupName,
+			ServerName:        name,
+			Location:          location,
+		}
+		availableResp, err := sdkClient.CheckNameAvailability()
+		if err != nil {
+			log.Info("error validating name")
+			return ctrl.Result{}, err
+		}
+		if !availableResp.Available {
+			log.Info("Servername is invalid or not available")
+			r.Recorder.Event(&instance, "Warning", "Failed", "Servername is invalid")
+			return ctrl.Result{Requeue: false}, fmt.Errorf("Servername invalid %s", availableResp.Name)
+		}
+	*/
 
 	// availableResp, err := sdkClient.CheckNameAvailability()
 	// if err != nil {
@@ -272,15 +272,6 @@ 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
 	}