add tests, remove commented code

Makefile

@@ -24,6 +24,7 @@ EFFECTIVE_VERSION           := $(VERSION)-$(shell git rev-parse HEAD)
 LD_FLAGS                    := "-w $(shell $(REPO_ROOT)/vendor/github.com/gardener/gardener/hack/get-build-ld-flags.sh k8s.io/component-base $(REPO_ROOT)/VERSION $(EXTENSION_PREFIX))"
 LEADER_ELECTION             := false
 IGNORE_OPERATION_ANNOTATION := true
+PLATFORM 					:= linux/amd64
 TEST_RECONCILER             := tf
 
 WEBHOOK_CONFIG_PORT	:= 8443
@@ -98,8 +99,8 @@ docker-login:
 
 .PHONY: docker-images
 docker-images:
-	@docker buildx build --platform linux/amd64 --build-arg EFFECTIVE_VERSION=$(EFFECTIVE_VERSION) -t $(IMAGE_PREFIX)/$(NAME):$(VERSION)           -t $(IMAGE_PREFIX)/$(NAME):latest           -f Dockerfile -m 6g --target $(EXTENSION_PREFIX)-$(NAME)           .
-	@docker buildx build --platform linux/amd64 --build-arg EFFECTIVE_VERSION=$(EFFECTIVE_VERSION) -t $(IMAGE_PREFIX)/$(ADMISSION_NAME):$(VERSION) -t $(IMAGE_PREFIX)/$(ADMISSION_NAME):latest -f Dockerfile -m 6g --target $(EXTENSION_PREFIX)-$(ADMISSION_NAME) .
+	@docker buildx build --platform $(PLATFORM) --build-arg EFFECTIVE_VERSION=$(EFFECTIVE_VERSION) -t $(IMAGE_PREFIX)/$(NAME):$(VERSION) -t $(IMAGE_PREFIX)/$(NAME):latest -f Dockerfile -m 6g --target $(EXTENSION_PREFIX)-$(NAME) .
+	@docker buildx build --platform $(PLATFORM) --build-arg EFFECTIVE_VERSION=$(EFFECTIVE_VERSION) -t $(IMAGE_PREFIX)/$(ADMISSION_NAME):$(VERSION) -t $(IMAGE_PREFIX)/$(ADMISSION_NAME):latest -f Dockerfile -m 6g --target $(EXTENSION_PREFIX)-$(ADMISSION_NAME) .
 
 #####################################################################
 # Rules for verification, formatting, linting, testing and cleaning #

pkg/controller/infrastructure/infraflow/const.go

@@ -17,6 +17,8 @@ package infraflow
 const (
 	// ChildKeyIDs is the prefix key for all ids.
 	ChildKeyIDs = "ids"
+	// ChildKeyInventory is the prefix key for for the inventory struct.
+	ChildKeyInventory = "inventory"
 	// CreatedResourcesExistKey is a marker for the Terraform migration case. If the TF state is not empty
 	// we inject this marker into the state to block the deletion without having first a successful reconciliation.
 	CreatedResourcesExistKey = "resources_exist"

pkg/controller/infrastructure/infraflow/ensurer.go

@@ -69,7 +69,9 @@ func (f *FlowContext) ensureResourceGroup(ctx context.Context) (*armresources.Re
 	if rg != nil {
 		if location := pointer.StringDeref(rg.Location, ""); location != rgCfg.Location {
 			// special case - return an error but do not proceed without user input.
-			return nil, NewTerminalSpecMismatch(rgCfg.AzureResourceMetadata, "location", rgCfg.Location, location)
+			return nil, NewSpecMismatchError(rgCfg.AzureResourceMetadata, "location", rgCfg.Location, location,
+				to.Ptr("This error is caused because the resource group location does not match the shoot's region. To proceed please delete the resource group"),
+			)
 		}
 
 		return rg, nil
@@ -129,7 +131,7 @@ func (f *FlowContext) ensureManagedVirtualNetwork(ctx context.Context) (*armnetw
 
 	if vnet != nil {
 		if location := pointer.StringDeref(vnet.Location, ""); location != f.adapter.Region() {
-			log.Error(NewTerminalSpecMismatch(vnetCfg.AzureResourceMetadata, "location", f.adapter.Region(), location), "vnet can't be reconciled and has to be deleted")
+			log.Error(NewSpecMismatchError(vnetCfg.AzureResourceMetadata, "location", f.adapter.Region(), location, nil), "vnet can't be reconciled and has to be deleted")
 			err = c.Delete(ctx, vnetCfg.ResourceGroup, vnetCfg.Name)
 			if err != nil {
 				return nil, err
@@ -140,8 +142,8 @@ func (f *FlowContext) ensureManagedVirtualNetwork(ctx context.Context) (*armnetw
 	}
 
 	vnet = vnetCfg.ToProvider(vnet)
-	log.V(2).Info("creating virtual network", "name", vnetCfg.Name)
-	log.V(5).Info("virtual network", "spec", *vnet)
+	log.V(2).Info("reconciling virtual network", "name", vnetCfg.Name)
+	log.V(5).Info("creating virtual network with spec", "spec", *vnet)
 	vnet, err = c.CreateOrUpdate(ctx, vnetCfg.ResourceGroup, vnetCfg.Name, *vnet)
 	if err != nil {
 		return nil, err
@@ -167,6 +169,7 @@ func (f *FlowContext) ensureUserVirtualNetwork(ctx context.Context) (*armnetwork
 	if vnet == nil {
 		return nil, NewTerminalConditionError(vnetCfg.AzureResourceMetadata, fmt.Errorf("user vnet not found"))
 	}
+
 	log.V(5).Info("found user virtual network", "name", vnetCfg.Name)
 	return vnet, nil
 }
@@ -207,14 +210,14 @@ func (f *FlowContext) ensureAvailabilitySet(ctx context.Context, log logr.Logger
 
 	if avset != nil {
 		if location := pointer.StringDeref(avset.Location, ""); location != f.adapter.Region() {
-			log.Error(NewTerminalSpecMismatch(avsetCfg.AzureResourceMetadata, "location", f.adapter.Region(), location), "will attempt to delete availability set due to irreconcilable error")
+			log.Error(NewSpecMismatchError(avsetCfg.AzureResourceMetadata, "location", f.adapter.Region(), location, nil), "will attempt to delete availability set due to irreconcilable error")
 			err = asClient.Delete(ctx, avsetCfg.ResourceGroup, avsetCfg.Name)
 			if err != nil {
 				return nil, err
 			}
 		}
 
-		// domain counts are immutable, therefore we live with whatever is currently present.
+		// domain counts are immutable, therefore we need live with whatever is currently present.
 		return avset, nil
 	}
 
@@ -227,8 +230,8 @@ func (f *FlowContext) ensureAvailabilitySet(ctx context.Context, log logr.Logger
 		},
 		SKU: &armcompute.SKU{Name: to.Ptr(string(armcompute.AvailabilitySetSKUTypesAligned))}, // equal to managed = True in tf
 	}
-	log.V(2).Info("creating availability set", "name", avset.Name)
-	log.V(5).Info("creating availability set", "spec", *avset)
+	log.V(2).Info("reconciling availability set", "name", avset.Name)
+	log.V(5).Info("reconciling availability set", "spec", *avset)
 	return asClient.CreateOrUpdate(ctx, f.adapter.ResourceGroupName(), avsetCfg.Name, *avset)
 }
 
@@ -262,18 +265,18 @@ func (f *FlowContext) ensureRouteTable(ctx context.Context) (*armnetwork.RouteTa
 
 	if rt != nil {
 		if location := pointer.StringDeref(rt.Location, ""); location != f.adapter.Region() {
-			return nil, NewTerminalSpecMismatch(rtCfg.AzureResourceMetadata, "location", f.adapter.Region(), location)
+			log.Error(NewSpecMismatchError(rtCfg.AzureResourceMetadata, "location", f.adapter.Region(), location, nil), "will attempt to delete route table due to irreconcilable error")
+			err = c.Delete(ctx, rtCfg.ResourceGroup, rtCfg.Name)
+			if err != nil {
+				return nil, err
+			}
+			rt = nil
 		}
 	}
 
-	// create the RouteTable
-	rt = &armnetwork.RouteTable{
-		Location:   to.Ptr(f.adapter.Region()),
-		Properties: &armnetwork.RouteTablePropertiesFormat{},
-	}
-	log.V(2).Info("creating route table", "name", rtCfg.Name)
-	log.V(5).Info("creating route table with spec", "spec", *rt)
-
+	rt = rtCfg.ToProvider(rt)
+	log.V(2).Info("reconciling route table", "name", rtCfg.Name)
+	log.V(5).Info("reconciling route table with spec", "spec", *rt)
 	return c.CreateOrUpdate(ctx, rtCfg.ResourceGroup, rtCfg.Name, *rt)
 }
 
@@ -310,18 +313,18 @@ func (f *FlowContext) ensureSecurityGroup(ctx context.Context) (*armnetwork.Secu
 
 	if sg != nil {
 		if location := pointer.StringDeref(sg.Location, ""); location != f.adapter.Region() {
-			return nil, NewTerminalSpecMismatch(sgCfg.AzureResourceMetadata, "location", f.adapter.Region(), location)
+			log.Error(NewSpecMismatchError(sgCfg.AzureResourceMetadata, "location", f.adapter.Region(), location, nil), "will attempt to delete security group due to irreconcilable error")
+			err = c.Delete(ctx, sgCfg.ResourceGroup, sgCfg.Name)
+			if err != nil {
+				return nil, err
+			}
+			sg = nil
 		}
-		return sg, nil
 	}
 
-	// create the NSG if it not there
-	sg = &armnetwork.SecurityGroup{
-		Location:   to.Ptr(f.adapter.Region()),
-		Properties: &armnetwork.SecurityGroupPropertiesFormat{},
-	}
-	log.V(2).Info("creating security group", "name", sgCfg.Name)
-	log.V(5).Info("creating security group with spec", "spec", *sg)
+	sg = sgCfg.ToProvider(sg)
+	log.V(2).Info("reconciling security group", "name", sgCfg.Name)
+	log.V(5).Info("reconciling security group with spec", "spec", *sg)
 	sg, err = c.CreateOrUpdate(ctx, sgCfg.ResourceGroup, sgCfg.Name, *sg)
 	if err != nil {
 		return nil, err
@@ -412,7 +415,7 @@ func (f *FlowContext) ensurePublicIps(ctx context.Context) error {
 
 		// delete all resources whose spec cannot be updated to match target spec.
 		if ok, offender, v := ForceNewIp(current, toReconcile[pipCfg.Name]); ok {
-			log.Info("will delete public IP because it can't be reconciled", "Resource Group", f.adapter.ResourceGroupName(), "Name", name, "Offender", offender, "Value", v)
+			log.Info("will delete public IP because it can't be reconciled", "Resource Group", f.adapter.ResourceGroupName(), "Name", name, "Field", offender, "Value", v)
 			toDelete[name] = *current.ID
 			continue
 		}
@@ -508,7 +511,7 @@ func (f *FlowContext) ensureNatGateways(ctx context.Context) error {
 			continue
 		}
 		if ok, offender, v := ForceNewNat(current, targetNat); ok {
-			log.Info("will delete NAT Gateway because it cannot be reconciled", "Resource Group", f.adapter.ResourceGroupName(), "Name", *current.Name, "Offender", offender, "Value", v)
+			log.Info("will delete NAT Gateway because it cannot be reconciled", "Resource Group", f.adapter.ResourceGroupName(), "Name", *current.Name, "Field", offender, "Value", v)
 			toDelete[name] = *current.ID
 			continue
 		}
@@ -607,7 +610,7 @@ func (f *FlowContext) ensureSubnets(ctx context.Context) (err error) {
 			continue
 		}
 		if ok, offender, v := ForceNewSubnet(current, target); ok {
-			log.Info("will delete subnet because it cannot be reconciled", "Resource Group", vnetRgroup, "Name", *current.Name, "Offender", offender, "Value", v)
+			log.Info("will delete subnet because it cannot be reconciled", "Resource Group", vnetRgroup, "Name", *current.Name, "Field", offender, "Value", v)
 			toDelete[name] = current
 			continue
 		}
@@ -665,7 +668,7 @@ func (f *FlowContext) EnsureManagedIdentity(ctx context.Context) (err error) {
 }
 
 // GetInfrastructureStatus returns the infrastructure status.
-func (f *FlowContext) GetInfrastructureStatus(ctx context.Context) (*v1alpha1.InfrastructureStatus, error) {
+func (f *FlowContext) GetInfrastructureStatus(_ context.Context) (*v1alpha1.InfrastructureStatus, error) {
 	status := &v1alpha1.InfrastructureStatus{
 		TypeMeta: infrastructure.StatusTypeMeta,
 		Networks: v1alpha1.NetworkStatus{
@@ -722,9 +725,12 @@ func (f *FlowContext) GetInfrastructureStatus(ctx context.Context) (*v1alpha1.In
 		}
 	}
 
-	err := f.enrichStatusWithIdentity(ctx, status)
-	if err != nil {
-		return status, err
+	if identity := f.cfg.Identity; identity != nil {
+		status.Identity = &v1alpha1.IdentityStatus{
+			ID:        *f.whiteboard.Get(KeyManagedIdentityId),
+			ClientID:  *f.whiteboard.Get(KeyManagedIdentityClientId),
+			ACRAccess: identity.ACRAccess != nil && *identity.ACRAccess,
+		}
 	}
 
 	return status, nil

pkg/controller/infrastructure/infraflow/ensurer_errors.go

@@ -18,22 +18,32 @@ import (
 	"fmt"
 )
 
-// TerminalSpecMismatchError is an error to indicate that the reconciliation cannot proceed or the operation requested is not supported.
-type TerminalSpecMismatchError struct {
+// SpecMismatchError is an error to indicate that the reconciliation cannot proceed or the operation requested is not supported.
+type SpecMismatchError struct {
+	// AzureResourceMetadata describe uniquely an Azure resource
 	AzureResourceMetadata
-	Offender string
+	// Field is the name of field that could not be reconciled.
+	Field string
+	// Expected is the value of the field that was expected.
 	Expected any
-	Found    any
+	// Found is the actual value of Field.
+	Found any
+	// Info contains additional information or instruction to the user.
+	Info *string
 }
 
-// NewTerminalSpecMismatch creates a TerminalSpecMismatch error.
-func NewTerminalSpecMismatch(identifier AzureResourceMetadata, offender string, expected, found any) *TerminalSpecMismatchError {
-	return &TerminalSpecMismatchError{AzureResourceMetadata: identifier, Offender: offender, Expected: expected, Found: found}
+// NewSpecMismatchError creates a TerminalSpecMismatch error.
+func NewSpecMismatchError(identifier AzureResourceMetadata, offender string, expected, found any, info *string) *SpecMismatchError {
+	return &SpecMismatchError{AzureResourceMetadata: identifier, Field: offender, Expected: expected, Found: found, Info: info}
 }
 
-func (t *TerminalSpecMismatchError) Error() string {
-	return fmt.Sprintf("differences between the current and target spec require the object to be deleted, but "+
-		"the operation is not supported. Resource: %s, Name: %s, Field: %s, Expected: %v, Found: %v", t.Kind, t.Name, t.Offender, t.Expected, t.Found)
+func (t *SpecMismatchError) Error() string {
+	s := fmt.Sprintf("differences between the current and target spec require the object to be deleted."+
+		"Resource: %s, Name: %s, Field: %s, Expected: %v, Found: %v", t.Kind, t.Name, t.Field, t.Expected, t.Found)
+	if t.Info != nil {
+		s = fmt.Sprintf("%s. Additional info: %s", s, *t.Info)
+	}
+	return s
 }
 
 // TerminalConditionError is an error to mark cases where the reconciliation cannot continue.
@@ -42,7 +52,7 @@ type TerminalConditionError struct {
 	error
 }
 
-// NewTerminalConditionError creates a TerminalConditinoError.
+// NewTerminalConditionError creates a TerminalConditionError.
 func NewTerminalConditionError(identifier AzureResourceMetadata, err error) *TerminalConditionError {
 	return &TerminalConditionError{identifier, err}
 }

pkg/controller/infrastructure/infraflow/ensurer_helper.go

@@ -16,7 +16,6 @@ package infraflow
 
 import (
 	"strings"
-	"sync"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
 
@@ -79,73 +78,72 @@ func Join[K comparable, V any](m1, m2 map[K]V) map[K]V {
 	return m1
 }
 
-// SimpleInventory is responsible for managing a list of all infrastructure created objects.
-type SimpleInventory struct {
-	sync.Mutex
-	inventory map[string]*arm.ResourceID
+// Inventory is responsible for managing a list of all infrastructure created objects.
+type Inventory struct {
+	shared.Whiteboard
 }
 
-// NewSimpleInventory returns a new instance of SimpleInventory.
-func NewSimpleInventory() *SimpleInventory {
-	return &SimpleInventory{
-		inventory: map[string]*arm.ResourceID{},
-	}
+// NewSimpleInventory returns a new instance of Inventory.
+func NewSimpleInventory(wb shared.Whiteboard) *Inventory {
+	return &Inventory{wb}
 }
 
 // Insert inserts the id to the inventory.
-func (i *SimpleInventory) Insert(id string) error {
-	i.Lock()
-	defer i.Unlock()
+func (i *Inventory) Insert(id string) error {
 	resourceID, err := arm.ParseResourceID(id)
 	if err != nil {
 		return err
 	}
 
-	i.inventory[id] = resourceID
+	i.GetChild(ChildKeyInventory).SetObject(id, resourceID)
+	return nil
+}
+
+// Get gets the item from the inventory.
+func (i *Inventory) Get(id string) *arm.ResourceID {
+	if i.GetChild(ChildKeyInventory).HasObject(id) {
+		return i.GetChild(ChildKeyInventory).GetObject(id).(*arm.ResourceID)
+	}
 	return nil
 }
 
 // Delete deletes the item with ID==id from the inventory and any children it may have. That means that it deletes any ID prefixed by id,
 // since azure IDs are hierarchical.
-func (i *SimpleInventory) Delete(id string) {
-	i.Lock()
-	defer i.Unlock()
-	delete(i.inventory, id)
-
+func (i *Inventory) Delete(id string) {
 	// since azure IDs are hierarchical, we remove from our inventory all items that are prefixed by the Id we want to remove.
-	for k := range i.inventory {
-		if strings.HasPrefix(k, id) {
-			delete(i.inventory, id)
+	for _, key := range i.GetChild(ChildKeyInventory).ObjectKeys() {
+		if strings.HasPrefix(key, id) {
+			i.GetChild(ChildKeyInventory).DeleteObject(key)
 		}
 	}
 }
 
 // ByKind returns a list of all the IDs of stored objects of a particular kind.
-func (i *SimpleInventory) ByKind(kind AzureResourceKind) []string {
-	i.Lock()
-	defer i.Unlock()
+func (i *Inventory) ByKind(kind AzureResourceKind) []string {
 	res := make([]string, 0)
-	for _, v := range i.inventory {
-		if v.ResourceType.String() == kind.String() {
-			res = append(res, v.Name)
-
+	for _, key := range i.GetChild(ChildKeyInventory).ObjectKeys() {
+		if i.GetChild(ChildKeyInventory).HasObject(key) {
+			resource := i.GetChild(ChildKeyInventory).GetObject(key).(*arm.ResourceID)
+			if resource.ResourceType.String() == kind.String() {
+				res = append(res, resource.Name)
+			}
 		}
 	}
 
 	return res
 }
 
 // ToList returns a list of v1alpha1 API objects that correspond to the current inventory list.
-func (i *SimpleInventory) ToList() []v1alpha1.AzureResource {
-	i.Lock()
-	defer i.Unlock()
-
+func (i *Inventory) ToList() []v1alpha1.AzureResource {
 	var res []v1alpha1.AzureResource
-	for k, v := range i.inventory {
-		res = append(res, v1alpha1.AzureResource{
-			Kind: v.ResourceType.String(),
-			ID:   k,
-		})
+	for _, key := range i.GetChild(ChildKeyInventory).ObjectKeys() {
+		if i.GetChild(ChildKeyInventory).HasObject(key) {
+			resource := i.GetChild(ChildKeyInventory).GetObject(key).(*arm.ResourceID)
+			res = append(res, v1alpha1.AzureResource{
+				Kind: resource.ResourceType.String(),
+				ID:   key,
+			})
+		}
 	}
 	return res
 }