Add NSG extension to merge SecurityRules

v2/api/network/customizations/load_balancer_extension.go

@@ -4,7 +4,6 @@ package customizations
 
 import (
 	"context"
-	"encoding/json"
 	"reflect"
 
 	"github.com/go-logr/logr"
@@ -21,8 +20,9 @@ import (
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
 )
 
-// Attention: A lot of code in this file is very similar to the logic in route_table_extensions.go and virtual_network_extensions.go.
+// Attention: A lot of code in this file is very similar to the logic in network_security_group_extension.go, route_table_extensions.go and virtual_network_extensions.go.
 // The two should be kept in sync as much as possible.
+// NOTE: This wouldn't work without adding indexes in 'getGeneratedStorageTypes' method in controller_resources.go
 
 var _ extensions.ARMResourceModifier = &LoadBalancerExtension{}
 
@@ -66,7 +66,7 @@ func (extension *LoadBalancerExtension) ModifyARMResource(
 
 	log.V(Info).Info("Found InboundNatRules to include on LoadBalancer", "count", len(armInboundNatRules), "names", genruntime.ARMSpecNames(armInboundNatRules))
 
-	err = fuzzySetInboundNatRules(armObj.Spec(), armInboundNatRules)
+	err = fuzzySetResources(armObj.Spec(), armInboundNatRules, "InboundNatRules")
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to set InboundNatRules")
 	}
@@ -80,94 +80,3 @@ func getInboundNatRuleGVK(lb genruntime.ARMMetaObject) schema.GroupVersionKind {
 
 	return gvk
 }
-
-// fuzzySetSubnets assigns a collection of subnets to the subnets property of the loadBalancer. Since there are
-// many possible ARM API versions and we don't know which one we're using, we cannot do this statically.
-// To make matters even more horrible, the type used in the loadBalancer.properties.inboundNatRule property is not the same
-// type as used for inboundNatRule.properties (although structurally they are basically the same). To overcome this
-// we JSON serialize the subnet and deserialize it into the loadBalancer.properties.inboundNatRule field.
-func fuzzySetInboundNatRules(lb genruntime.ARMResourceSpec, inboundNatRules []genruntime.ARMResourceSpec) (err error) {
-	if len(inboundNatRules) == 0 {
-		// Nothing to do
-		return nil
-	}
-
-	defer func() {
-		if x := recover(); x != nil {
-			err = errors.Errorf("caught panic: %s", x)
-		}
-	}()
-
-	// Here be dragons
-	lbValue := reflect.ValueOf(lb)
-	lbValue = reflect.Indirect(lbValue)
-	if (lbValue == reflect.Value{}) {
-		return errors.Errorf("cannot assign to nil loadbalancer")
-	}
-
-	propertiesField := lbValue.FieldByName("Properties")
-	if (propertiesField == reflect.Value{}) {
-		return errors.Errorf("couldn't find properties field on loadbalancer")
-	}
-
-	propertiesValue := reflect.Indirect(propertiesField)
-	if (propertiesValue == reflect.Value{}) {
-		// If the properties field is nil, we must construct an entirely new properties and assign it here
-		temp := reflect.New(propertiesField.Type().Elem())
-		propertiesField.Set(temp)
-		propertiesValue = reflect.Indirect(temp)
-	}
-
-	inboundNatRulesField := propertiesValue.FieldByName("InboundNatRules")
-	if (inboundNatRulesField == reflect.Value{}) {
-		return errors.Errorf("couldn't find InboundNatRules field on LoadBalancer")
-	}
-
-	if inboundNatRulesField.Type().Kind() != reflect.Slice {
-		return errors.Errorf("inboundNatRules field on LoadBalancer was not of kind Slice")
-	}
-
-	elemType := inboundNatRulesField.Type().Elem()
-	inboundNatRuleSlice := reflect.MakeSlice(inboundNatRulesField.Type(), 0, 0)
-
-	for _, inboundNatRule := range inboundNatRules {
-		embeddedInboundNatRule := reflect.New(elemType)
-		err := fuzzySetInboundNatRule(inboundNatRule, embeddedInboundNatRule)
-		if err != nil {
-			return err
-		}
-
-		inboundNatRuleSlice = reflect.Append(inboundNatRuleSlice, reflect.Indirect(embeddedInboundNatRule))
-	}
-
-	// Now do the assignment. We do it differently here, as we need to make sure to retain current/updated/deleted InboundNatRules present on the LoadBalancer.
-	inboundNatRulesField.Set(reflect.AppendSlice(inboundNatRulesField, inboundNatRuleSlice))
-
-	return nil
-}
-
-func fuzzySetInboundNatRule(inboundNatRule genruntime.ARMResourceSpec, embeddedInboundNatRule reflect.Value) error {
-	inboundNatRuleJSON, err := json.Marshal(inboundNatRule)
-	if err != nil {
-		return errors.Wrapf(err, "failed to marshal inboundNatRule json")
-	}
-
-	err = json.Unmarshal(inboundNatRuleJSON, embeddedInboundNatRule.Interface())
-	if err != nil {
-		return errors.Wrapf(err, "failed to unmarshal inboundNatRule JSON")
-	}
-
-	// Safety check that these are actually the same:
-	// We can't use reflect.DeepEqual because the types are not the same.
-	embeddedInboundNatRuleJSON, err := json.Marshal(embeddedInboundNatRule.Interface())
-	if err != nil {
-		return errors.Wrap(err, "unable to check that embedded inboundNatRule is the same as inboundNatRule")
-	}
-
-	err = fuzzyEqualityComparison(embeddedInboundNatRuleJSON, inboundNatRuleJSON)
-	if err != nil {
-		return errors.Wrap(err, "failed during comparison for embeddedInboundNatRuleJSON and inboundNatRuleJSON")
-	}
-
-	return nil
-}

v2/api/network/customizations/load_balancer_extension_test.go

@@ -43,7 +43,7 @@ func Test_FuzzySetLoadBalancers(t *testing.T) {
 		},
 	}
 
-	err := fuzzySetInboundNatRules(lb, []genruntime.ARMResourceSpec{rule})
+	err := fuzzySetResources(lb, []genruntime.ARMResourceSpec{rule}, "InboundNatRules")
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(lb.Location).To(Equal(to.Ptr("westus")))
 	g.Expect(lb.Properties).ToNot(BeNil())

v2/api/network/customizations/network_security_group_extension.go

@@ -0,0 +1,82 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+package customizations
+
+import (
+	"context"
+	"reflect"
+
+	"github.com/go-logr/logr"
+	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/conversion"
+
+	network "github.com/Azure/azure-service-operator/v2/api/network/v1api20201101storage"
+	. "github.com/Azure/azure-service-operator/v2/internal/logging"
+	"github.com/Azure/azure-service-operator/v2/internal/resolver"
+	"github.com/Azure/azure-service-operator/v2/internal/util/kubeclient"
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
+)
+
+// Attention: A lot of code in this file is very similar to the logic in load_balancer_extension.go, route_table_extensions.go and virtual_network_extensions.go.
+// The two should be kept in sync as much as possible.
+// NOTE: This wouldn't work without adding indexes in 'getGeneratedStorageTypes' method in controller_resources.go
+
+var _ extensions.ARMResourceModifier = &NetworkSecurityGroupExtension{}
+
+func (extension *NetworkSecurityGroupExtension) ModifyARMResource(
+	ctx context.Context,
+	armObj genruntime.ARMResource,
+	obj genruntime.ARMMetaObject,
+	kubeClient kubeclient.Client,
+	resolver *resolver.Resolver,
+	log logr.Logger,
+) (genruntime.ARMResource, error) {
+	typedObj, ok := obj.(*network.NetworkSecurityGroup)
+	if !ok {
+		return nil, errors.Errorf("cannot run on unknown resource type %T, expected *network.NetworkSecurityGroup", obj)
+	}
+
+	// Type assert that we are the hub type. This will fail to compile if
+	// the hub type has been changed but this extension has not been updated to match
+	var _ conversion.Hub = typedObj
+
+	networkSecurityGroupsSecurityRuleGVK := getNetworkSecurityGroupsSecurityRuleGVK(obj)
+
+	networkSecurityGroupsSecurityRules := &network.NetworkSecurityGroupsSecurityRuleList{}
+	matchingFields := client.MatchingFields{".metadata.ownerReferences[0]": string(obj.GetUID())}
+	err := kubeClient.List(ctx, networkSecurityGroupsSecurityRules, matchingFields)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed listing NetworkSecurityGroupsSecurityRule owned by NetworkSecurityGroup %s/%s", obj.GetNamespace(), obj.GetName())
+	}
+
+	armNetworkSecurityGroupsSecurityRule := make([]genruntime.ARMResourceSpec, 0, len(networkSecurityGroupsSecurityRules.Items))
+	for _, networkSecurityGroupsSecurityRule := range networkSecurityGroupsSecurityRules.Items {
+		networkSecurityGroupsSecurityRule := networkSecurityGroupsSecurityRule
+
+		var transformed genruntime.ARMResourceSpec
+		transformed, err = transformToARM(ctx, &networkSecurityGroupsSecurityRule, networkSecurityGroupsSecurityRuleGVK, kubeClient, resolver)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to transform NetworkSecurityGroupsSecurityRules %s/%s", networkSecurityGroupsSecurityRule.GetNamespace(), networkSecurityGroupsSecurityRule.GetName())
+		}
+		armNetworkSecurityGroupsSecurityRule = append(armNetworkSecurityGroupsSecurityRule, transformed)
+	}
+
+	log.V(Info).Info("Found NetworkSecurityGroupsSecurityRule to include on NetworkSecurityGroup", "count", len(armNetworkSecurityGroupsSecurityRule), "names", genruntime.ARMSpecNames(armNetworkSecurityGroupsSecurityRule))
+
+	err = fuzzySetResources(armObj.Spec(), armNetworkSecurityGroupsSecurityRule, "SecurityRules")
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to set networkSecurityGroupsSecurityRules")
+	}
+
+	return armObj, nil
+}
+
+func getNetworkSecurityGroupsSecurityRuleGVK(nsg genruntime.ARMMetaObject) schema.GroupVersionKind {
+	gvk := genruntime.GetOriginalGVK(nsg)
+	gvk.Kind = reflect.TypeOf(network.NetworkSecurityGroupsSecurityRule{}).Name() // "NetworkSecurityGroupsSecurityRule"
+
+	return gvk
+}

v2/api/network/customizations/network_security_group_extension_test.go

@@ -0,0 +1,50 @@
+/*
+Copyright (c) Microsoft Corporation.
+Licensed under the MIT license.
+*/
+
+package customizations
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+
+	network "github.com/Azure/azure-service-operator/v2/api/network/v1api20201101"
+	"github.com/Azure/azure-service-operator/v2/internal/util/to"
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
+)
+
+func Test_FuzzySetNetworkSecurityGroupsSecurityRules(t *testing.T) {
+	t.Parallel()
+	g := NewGomegaWithT(t)
+
+	nsg := &network.NetworkSecurityGroup_Spec_ARM{
+		Location: to.Ptr("westus"),
+		Name:     "my-nsg",
+	}
+
+	rule := &network.NetworkSecurityGroups_SecurityRule_Spec_ARM{
+		Name: "myrule",
+		Properties: &network.SecurityRulePropertiesFormat_NetworkSecurityGroups_SecurityRule_SubResourceEmbedded_ARM{
+			Access:                   to.Ptr(network.SecurityRuleAccess_Allow),
+			DestinationAddressPrefix: to.Ptr("*"),
+			DestinationPortRange:     to.Ptr("46-56"),
+			Direction:                to.Ptr(network.SecurityRuleDirection_Inbound),
+			Priority:                 to.Ptr(123),
+			Protocol:                 to.Ptr(network.SecurityRulePropertiesFormat_Protocol_Tcp),
+			SourceAddressPrefix:      to.Ptr("*"),
+			SourcePortRange:          to.Ptr("23-45"),
+		},
+	}
+
+	err := fuzzySetResources(nsg, []genruntime.ARMResourceSpec{rule}, "SecurityRules")
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(nsg.Location).To(Equal(to.Ptr("westus")))
+	g.Expect(nsg.Properties).ToNot(BeNil())
+	g.Expect(nsg.Properties.SecurityRules).To(HaveLen(1))
+	g.Expect(nsg.Properties.SecurityRules[0].Properties).ToNot(BeNil())
+	g.Expect(nsg.Properties.SecurityRules[0].Name).To(Equal(to.Ptr("myrule")))
+	g.Expect(nsg.Properties.SecurityRules[0].Properties.DestinationAddressPrefix).ToNot(BeNil())
+	g.Expect(nsg.Properties.SecurityRules[0].Properties.DestinationPortRange).ToNot(BeNil())
+}

v2/api/network/customizations/route_table_extensions.go

@@ -5,7 +5,6 @@ package customizations
 
 import (
 	"context"
-	"encoding/json"
 	"reflect"
 
 	"github.com/go-logr/logr"
@@ -22,8 +21,9 @@ import (
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
 )
 
-// Attention: A lot of code in this file is very similar to the logic in load_balancer_extensions.go and virtual_network_extensions.go.
+// Attention: A lot of code in this file is very similar to the logic in network_security_group_extension.go, load_balancer_extensions.go and virtual_network_extensions.go.
 // The two should be kept in sync as much as possible.
+// NOTE: This wouldn't work without adding indexes in 'getGeneratedStorageTypes' method in controller_resources.go
 
 var _ extensions.ARMResourceModifier = &RouteTableExtension{}
 
@@ -67,7 +67,7 @@ func (extension *RouteTableExtension) ModifyARMResource(
 
 	log.V(Info).Info("Found routes to include on RouteTable", "count", len(armRoutes), "names", genruntime.ARMSpecNames(armRoutes))
 
-	err = fuzzySetRoutes(armObj.Spec(), armRoutes)
+	err = fuzzySetResources(armObj.Spec(), armRoutes, "Routes")
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to set routes")
 	}
@@ -81,97 +81,3 @@ func getRouteGVK(routeTable genruntime.ARMMetaObject) schema.GroupVersionKind {
 
 	return gvk
 }
-
-// TODO: When we move to Swagger as the source of truth, the type for routetable.properties.routes and routtableroutes.properties
-// TODO: may be the same, so we can do away with the JSON serialization part of this assignment.
-// fuzzySetRoutes assigns a collection of routes to the routes property of the route table. Since there are
-// many possible ARM API versions and we don't know which one we're using, we cannot do this statically.
-// To make matters even more horrible, the type used in the routetable.properties.routes is not the same
-// type as used for routes.properties (although structurally they are basically the same). To overcome this
-// we JSON serialize the route and deserialize it into the routetableroutes.properties.routes field.
-func fuzzySetRoutes(routeTable genruntime.ARMResourceSpec, routes []genruntime.ARMResourceSpec) (err error) {
-	if len(routes) == 0 {
-		// Nothing to do
-		return nil
-	}
-
-	defer func() {
-		if x := recover(); x != nil {
-			err = errors.Errorf("caught panic: %s", x)
-		}
-	}()
-
-	// Here be dragons
-	routeTableValue := reflect.ValueOf(routeTable)
-	routeTableValue = reflect.Indirect(routeTableValue)
-
-	if (routeTableValue == reflect.Value{}) {
-		return errors.Errorf("cannot assign to nil route table")
-	}
-
-	propertiesField := routeTableValue.FieldByName("Properties")
-	if (propertiesField == reflect.Value{}) {
-		return errors.Errorf("couldn't find properties field on route table")
-	}
-
-	propertiesValue := reflect.Indirect(propertiesField)
-	if (propertiesValue == reflect.Value{}) {
-		// If the properties field is nil, we must construct an entirely new properties and assign it here
-		temp := reflect.New(propertiesField.Type().Elem())
-		propertiesField.Set(temp)
-		propertiesValue = reflect.Indirect(temp)
-	}
-
-	routesField := propertiesValue.FieldByName("Routes")
-	if (routesField == reflect.Value{}) {
-		return errors.Errorf("couldn't find routes field on route table")
-	}
-
-	if routesField.Type().Kind() != reflect.Slice {
-		return errors.Errorf("routes field on route table was not of kind Slice")
-	}
-
-	elemType := routesField.Type().Elem()
-	routesSlice := reflect.MakeSlice(routesField.Type(), 0, 0)
-
-	for _, route := range routes {
-		embeddedRoute := reflect.New(elemType)
-		err = fuzzySetRoute(route, embeddedRoute)
-		if err != nil {
-			return err
-		}
-
-		routesSlice = reflect.Append(routesSlice, reflect.Indirect(embeddedRoute))
-	}
-
-	// Now do the assignment
-	routesField.Set(routesSlice)
-
-	return err
-}
-
-func fuzzySetRoute(route genruntime.ARMResourceSpec, embeddedRoute reflect.Value) error {
-	routeJSON, err := json.Marshal(route)
-	if err != nil {
-		return errors.Wrapf(err, "failed to marshal route json")
-	}
-
-	err = json.Unmarshal(routeJSON, embeddedRoute.Interface())
-	if err != nil {
-		return errors.Wrapf(err, "failed to unmarshal route JSON")
-	}
-
-	// Safety check that these are actually the same:
-	// We can't use reflect.DeepEqual because the types are not the same.
-	embeddedRouteJSON, err := json.Marshal(embeddedRoute.Interface())
-	if err != nil {
-		return errors.Wrap(err, "unable to check that embedded route is the same as route")
-	}
-
-	err = fuzzyEqualityComparison(embeddedRouteJSON, routeJSON)
-	if err != nil {
-		return errors.Wrap(err, "failed during comparison for embeddedRouteJSON and routeJSON")
-	}
-
-	return nil
-}