refactor(scope): GetIdentityRefFromObjects

factorize identityRef lookup from multiple object to implement logic of identityRef's inheritage in one place (scope factory)

Signed-off-by: MatthieuFin <[email protected]>

Author: MatthieuFin

controllers/openstackmachine_controller.go

@@ -392,9 +392,12 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		Address: instanceStatus.Name(),
 	})
 	openStackMachine.Status.Addresses = addresses
-	if openStackMachine.Spec.IdentityRef == nil {
-		openStackMachine.Spec.IdentityRef = &openStackCluster.Spec.IdentityRef
-	}
+
+	_, identityRef := r.ScopeFactory.GetIdentityRefFromObjects(openStackMachine, openStackCluster)
+	openStackMachine.Spec.IdentityRef = identityRef
+
+	_, identityRef := r.ScopeFactory.GetIdentityRefFromObjects(openStackMachine, openStackCluster)
+	openStackMachine.Spec.IdentityRef = identityRef
 
 	result := r.reconcileMachineState(scope, openStackMachine, machine, machineServer)
 	if result != nil {
@@ -580,13 +583,11 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
 	}
 	if apierrors.IsNotFound(err) {
 		// Use credentials from the machine object by default, falling back to cluster credentials.
-		identityRef := func() infrav1.OpenStackIdentityReference {
-			if openStackMachine.Spec.IdentityRef != nil {
-				return *openStackMachine.Spec.IdentityRef
-			}
-			return openStackCluster.Spec.IdentityRef
-		}()
-		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
+		_, identityRef := r.ScopeFactory.GetIdentityRefFromObjects(openStackMachine, openStackCluster)
+		if identityRef == nil {
+			return nil, fmt.Errorf("unable to get identityRef from provided objects")
+		}
+		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, *identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
 		machineServer = &infrav1alpha1.OpenStackServer{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{

pkg/scope/mock.go

@@ -33,6 +33,7 @@ import (
 // MockScopeFactory implements both the ScopeFactory and ClientScope interfaces. It can be used in place of the default ProviderScopeFactory
 // when we want to use mocked service clients which do not attempt to connect to a running OpenStack cloud.
 type MockScopeFactory struct {
+	*defaultScopeFactory
 	ComputeClient *mock.MockComputeClient
 	NetworkClient *mock.MockNetworkClient
 	VolumeClient  *mock.MockVolumeClient

pkg/scope/provider.go

@@ -48,19 +48,12 @@ const (
 )
 
 type providerScopeFactory struct {
+	*defaultScopeFactory
 	clientCache *cache.LRUExpireCache
 }
 
 func (f *providerScopeFactory) NewClientScopeFromObject(ctx context.Context, ctrlClient client.Client, defaultCACert []byte, logger logr.Logger, objects ...infrav1.IdentityRefProvider) (Scope, error) {
-	var namespace *string
-	var identityRef *infrav1.OpenStackIdentityReference
-
-	for _, o := range objects {
-		namespace, identityRef = o.GetIdentityRef()
-		if namespace != nil || identityRef != nil {
-			break
-		}
-	}
+	namespace, identityRef := f.GetIdentityRefFromObjects(objects...)
 
 	if namespace == nil || identityRef == nil {
 		return nil, fmt.Errorf("unable to get identityRef from provided objects")

pkg/scope/scope.go

@@ -43,6 +43,21 @@ func NewFactory(maxCacheSize int) Factory {
 type Factory interface {
 	// NewClientScopeFromObject creates a new scope from the first object which returns an OpenStackIdentityRef
 	NewClientScopeFromObject(ctx context.Context, ctrlClient client.Client, defaultCACert []byte, logger logr.Logger, objects ...infrav1.IdentityRefProvider) (Scope, error)
+	GetIdentityRefFromObjects(objects ...infrav1.IdentityRefProvider) (*string, *infrav1.OpenStackIdentityReference)
+}
+
+type defaultScopeFactory struct{}
+
+func (f *defaultScopeFactory) GetIdentityRefFromObjects(objects ...infrav1.IdentityRefProvider) (*string, *infrav1.OpenStackIdentityReference) {
+	var namespace *string
+	var identityRef *infrav1.OpenStackIdentityReference
+	for _, o := range objects {
+		namespace, identityRef = o.GetIdentityRef()
+		if identityRef != nil {
+			break
+		}
+	}
+	return namespace, identityRef
 }
 
 // Scope contains arguments common to most operations.

test/e2e/shared/common.go

@@ -198,7 +198,7 @@ func (o OpenStackLogCollector) CollectMachineLog(ctx context.Context, management
 	if m.Spec.ProviderID == nil {
 		return fmt.Errorf("unable to get logs for machine since it has no provider ID")
 	}
-	providerID := getIDFromProviderID(*m.Spec.ProviderID)
+	providerID := GetIDFromProviderID(*m.Spec.ProviderID)
 
 	consolLog, err := GetOpenStackServerConsoleLog(o.E2EContext, providerID)
 	if err != nil {