ApplicationCredential Actuator and Status

On-behalf-of: SAP nils.gondermann@sap.com

Author: gndrmnn

internal/controllers/applicationcredential/actuator.go

@@ -29,7 +29,6 @@ import (
 	orcv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/generic/interfaces"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/generic/progress"
-	"github.com/k-orc/openstack-resource-controller/v2/internal/logging"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/osclients"
 	orcerrors "github.com/k-orc/openstack-resource-controller/v2/internal/util/errors"
 )
@@ -70,30 +69,42 @@ func (actuator applicationcredentialActuator) ListOSResourcesForAdoption(ctx con
 		return nil, false
 	}
 
-	// TODO(scaffolding) If you need to filter resources on fields that the List() function
-	// of gophercloud does not support, it's possible to perform client-side filtering.
-	// Check osclients.ResourceFilter
+	var filters []osclients.ResourceFilter[osResourceT]
+
+	// Add client-side filters
+	if resourceSpec.Description != nil {
+		filters = append(filters, func(f *applicationcredentials.ApplicationCredential) bool {
+			return f.Description == *resourceSpec.Description
+		})
+	}
 
 	listOpts := applicationcredentials.ListOpts{
-		Name:        getResourceName(orcObject),
-		Description: ptr.Deref(resourceSpec.Description, ""),
+		Name: getResourceName(orcObject),
 	}
 
-	return actuator.osClient.ListApplicationCredentials(ctx, listOpts), true
+	return actuator.listOSResources(ctx, resourceSpec.UserID, filters, listOpts), true
 }
 
 func (actuator applicationcredentialActuator) ListOSResourcesForImport(ctx context.Context, obj orcObjectPT, filter filterT) (iter.Seq2[*osResourceT, error], progress.ReconcileStatus) {
-	// TODO(scaffolding) If you need to filter resources on fields that the List() function
-	// of gophercloud does not support, it's possible to perform client-side filtering.
-	// Check osclients.ResourceFilter
+	var filters []osclients.ResourceFilter[osResourceT]
+
+	// Add client-side filters
+	if filter.Description != nil {
+		filters = append(filters, func(f *applicationcredentials.ApplicationCredential) bool {
+			return f.Description == *filter.Description
+		})
+	}
 
 	listOpts := applicationcredentials.ListOpts{
-		Name:        string(ptr.Deref(filter.Name, "")),
-		Description: string(ptr.Deref(filter.Description, "")),
-		// TODO(scaffolding): Add more import filters
+		Name: string(ptr.Deref(filter.Name, "")),
 	}
 
-	return actuator.osClient.ListApplicationCredentials(ctx, listOpts), nil
+	return actuator.listOSResources(ctx, filter.UserID, filters, listOpts), nil
+}
+
+func (actuator applicationcredentialActuator) listOSResources(ctx context.Context, userID string, filters []osclients.ResourceFilter[osResourceT], listOpts applicationcredentials.ListOptsBuilder) iter.Seq2[*applicationcredentials.ApplicationCredential, error] {
+	applicationCredentials := actuator.osClient.ListApplicationCredentials(ctx, userID, listOpts)
+	return osclients.Filter(applicationCredentials, filters...)
 }
 
 func (actuator applicationcredentialActuator) CreateResource(ctx context.Context, obj orcObjectPT) (*osResourceT, progress.ReconcileStatus) {
@@ -104,100 +115,71 @@ func (actuator applicationcredentialActuator) CreateResource(ctx context.Context
 		return nil, progress.WrapError(
 			orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "Creation requested, but spec.resource is not set"))
 	}
-	createOpts := applicationcredentials.CreateOpts{
-		Name:        getResourceName(obj),
-		Description: ptr.Deref(resource.Description, ""),
-		// TODO(scaffolding): Add more fields
-	}
-
-	osResource, err := actuator.osClient.CreateApplicationCredential(ctx, createOpts)
-	if err != nil {
-		// We should require the spec to be updated before retrying a create which returned a conflict
-		if !orcerrors.IsRetryable(err) {
-			err = orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "invalid configuration creating resource: "+err.Error(), err)
-		}
-		return nil, progress.WrapError(err)
-	}
 
-	return osResource, nil
-}
+	roleList := make([]applicationcredentials.Role, len(resource.Roles))
+	for i := range resource.Roles {
+		roleSpec := &resource.Roles[i]
+		role := &roleList[i]
 
-func (actuator applicationcredentialActuator) DeleteResource(ctx context.Context, _ orcObjectPT, resource *osResourceT) progress.ReconcileStatus {
-	return progress.WrapError(actuator.osClient.DeleteApplicationCredential(ctx, resource.ID))
-}
+		if roleSpec.ID != nil {
+			role.ID = *roleSpec.ID
+		}
 
-func (actuator applicationcredentialActuator) updateResource(ctx context.Context, obj orcObjectPT, osResource *osResourceT) progress.ReconcileStatus {
-	log := ctrl.LoggerFrom(ctx)
-	resource := obj.Spec.Resource
-	if resource == nil {
-		// Should have been caught by API validation
-		return progress.WrapError(
-			orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "Update requested, but spec.resource is not set"))
+		if roleSpec.Name != nil {
+			role.Name = string(*roleSpec.Name)
+		}
 	}
 
-	updateOpts := applicationcredentials.UpdateOpts{}
+	accessRuleList := make([]applicationcredentials.AccessRule, len(resource.AccessRules))
+	for i := range resource.AccessRules {
+		accessRuleSpec := &resource.AccessRules[i]
+		accessRule := &accessRuleList[i]
 
-	handleNameUpdate(&updateOpts, obj, osResource)
-	handleDescriptionUpdate(&updateOpts, resource, osResource)
+		if accessRuleSpec.Path != nil {
+			accessRule.Path = *accessRuleSpec.Path
+		}
 
-	// TODO(scaffolding): add handler for all fields supporting mutability
+		if accessRuleSpec.Service != nil {
+			accessRule.Service = *accessRuleSpec.Service
+		}
 
-	needsUpdate, err := needsUpdate(updateOpts)
-	if err != nil {
-		return progress.WrapError(
-			orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "invalid configuration updating resource: "+err.Error(), err))
-	}
-	if !needsUpdate {
-		log.V(logging.Debug).Info("No changes")
-		return nil
+		if accessRuleSpec.Method != nil {
+			accessRule.Method = string(*accessRuleSpec.Method)
+		}
 	}
 
-	_, err = actuator.osClient.UpdateApplicationCredential(ctx, osResource.ID, updateOpts)
-
-	// We should require the spec to be updated before retrying an update which returned a conflict
-	if orcerrors.IsConflict(err) {
-		err = orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "invalid configuration updating resource: "+err.Error(), err)
+	createOpts := applicationcredentials.CreateOpts{
+		Name:         getResourceName(obj),
+		Description:  ptr.Deref(resource.Description, ""),
+		Unrestricted: ptr.Deref(resource.Unrestricted, false),
+		Secret:       ptr.Deref(resource.Secret, ""),
+		Roles:        roleList,
+		AccessRules:  accessRuleList,
 	}
 
-	if err != nil {
-		return progress.WrapError(err)
+	if resource.ExpiresAt != nil {
+		createOpts.ExpiresAt = &resource.ExpiresAt.Time
 	}
 
-	return progress.NeedsRefresh()
-}
-
-func needsUpdate(updateOpts applicationcredentials.UpdateOpts) (bool, error) {
-	updateOptsMap, err := updateOpts.ToApplicationCredentialUpdateMap()
+	osResource, err := actuator.osClient.CreateApplicationCredential(ctx, resource.UserID, createOpts)
 	if err != nil {
-		return false, err
-	}
-
-	updateMap, ok := updateOptsMap["application_credentials"].(map[string]any)
-	if !ok {
-		updateMap = make(map[string]any)
+		// We should require the spec to be updated before retrying a create which returned a conflict
+		if !orcerrors.IsRetryable(err) {
+			err = orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "invalid configuration creating resource: "+err.Error(), err)
+		}
+		return nil, progress.WrapError(err)
 	}
 
-	return len(updateMap) > 0, nil
-}
-
-func handleNameUpdate(updateOpts *applicationcredentials.UpdateOpts, obj orcObjectPT, osResource *osResourceT) {
-	name := getResourceName(obj)
-	if osResource.Name != name {
-		updateOpts.Name = &name
-	}
+	return osResource, nil
 }
 
-func handleDescriptionUpdate(updateOpts *applicationcredentials.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {
-	description := ptr.Deref(resource.Description, "")
-	if osResource.Description != description {
-		updateOpts.Description = &description
-	}
+func (actuator applicationcredentialActuator) DeleteResource(ctx context.Context, orcObject orcObjectPT, resource *osResourceT) progress.ReconcileStatus {
+	return progress.WrapError(actuator.osClient.DeleteApplicationCredential(ctx, orcObject.Spec.Resource.UserID, resource.ID))
 }
 
 func (actuator applicationcredentialActuator) GetResourceReconcilers(ctx context.Context, orcObject orcObjectPT, osResource *osResourceT, controller interfaces.ResourceController) ([]resourceReconciler, progress.ReconcileStatus) {
-	return []resourceReconciler{
-		actuator.updateResource,
-	}, nil
+	// ApplicationCredentials are immutable - no update reconcilers needed
+	return []resourceReconciler{}, nil
 }
 
 type applicationcredentialHelperFactory struct{}

internal/controllers/applicationcredential/actuator_test.go

@@ -50,14 +50,55 @@ func (applicationcredentialStatusWriter) ResourceAvailableStatus(orcObject *orcv
 
 func (applicationcredentialStatusWriter) ApplyResourceStatus(log logr.Logger, osResource *osResourceT, statusApply *statusApplyT) {
 	resourceStatus := orcapplyconfigv1alpha1.ApplicationCredentialResourceStatus().
-		WithName(osResource.Name)
+		WithName(osResource.Name).
+		WithUnrestricted(osResource.Unrestricted).
+		WithProjectID(osResource.ProjectID)
 
-	// TODO(scaffolding): add all of the fields supported in the ApplicationCredentialResourceStatus struct
-	// If a zero-value isn't expected in the response, place it behind a conditional
+	if osResource.Secret != "" {
+		resourceStatus.WithSecret(osResource.Secret)
+	}
+
+	if !osResource.ExpiresAt.IsZero() {
+		resourceStatus.WithExpiresAt(metav1.NewTime(osResource.ExpiresAt))
+	}
+
+	if osResource.ID != "" {
+		resourceStatus.WithID(osResource.ID)
+	}
 
 	if osResource.Description != "" {
 		resourceStatus.WithDescription(osResource.Description)
 	}
 
+	for i := range osResource.Roles {
+		roleStatus := orcapplyconfigv1alpha1.ApplicationCredentialAccessRoleStatus().
+			WithID(osResource.Roles[i].ID).
+			WithName(osResource.Roles[i].Name)
+
+		if osResource.Roles[i].DomainID != "" {
+			roleStatus.WithDomainID(osResource.Roles[i].DomainID)
+		}
+
+		resourceStatus.WithRoles(roleStatus)
+	}
+
+	for i := range osResource.AccessRules {
+		accessRuleStatus := orcapplyconfigv1alpha1.ApplicationCredentialAccessRuleStatus().
+			WithID(osResource.AccessRules[i].ID).
+			WithPath(osResource.AccessRules[i].Path).
+			WithMethod(osResource.AccessRules[i].Method).
+			WithService(osResource.AccessRules[i].Service)
+
+		resourceStatus.WithAccessRules(accessRuleStatus)
+	}
+
+	links := make(map[string]string, len(osResource.Links))
+
+	for k, v := range osResource.Links {
+		links[k] = v.(string)
+	}
+
+	resourceStatus.WithLinks(links)
+
 	statusApply.WithResource(resourceStatus)
 }