diff --git a/api/v1alpha1/awsiamprovision_types.go b/api/v1alpha1/awsiamprovision_types.go index fe0e7b2..3ea702c 100644 --- a/api/v1alpha1/awsiamprovision_types.go +++ b/api/v1alpha1/awsiamprovision_types.go @@ -35,13 +35,23 @@ type AWSIAMProvisionSpec struct { Roles map[string]AWSIAMProvisionRole `json:"roles"` } +// AWSIAMProvisionStatusRole defines the observed state of AWSIAMProvision's roles. +type AWSIAMProvisionStatusRole struct { + // Important: Run "make" to regenerate code after modifying this file + + Message string `json:"message,omitempty"` + Phase string `json:"phase,omitempty"` + Status iamctrlv1alpha1.RoleStatus `json:"status,omitempty"` +} + // AWSIAMProvisionStatus defines the observed state of AWSIAMProvision. type AWSIAMProvisionStatus struct { // Important: Run "make" to regenerate code after modifying this file - Message string `json:"message,omitempty"` - LastUpdatedTime *metav1.Time `json:"lastUpdatedTime,omitempty"` - Phase string `json:"phase,omitempty"` + Message string `json:"message,omitempty"` + LastUpdatedTime *metav1.Time `json:"lastUpdatedTime,omitempty"` + Phase string `json:"phase,omitempty"` + Roles map[string]AWSIAMProvisionStatusRole `json:"roles,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 4e37b50..7491144 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -128,6 +128,13 @@ func (in *AWSIAMProvisionStatus) DeepCopyInto(out *AWSIAMProvisionStatus) { in, out := &in.LastUpdatedTime, &out.LastUpdatedTime *out = (*in).DeepCopy() } + if in.Roles != nil { + in, out := &in.Roles, &out.Roles + *out = make(map[string]AWSIAMProvisionStatusRole, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSIAMProvisionStatus. @@ -139,3 +146,19 @@ func (in *AWSIAMProvisionStatus) DeepCopy() *AWSIAMProvisionStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AWSIAMProvisionStatusRole) DeepCopyInto(out *AWSIAMProvisionStatusRole) { + *out = *in + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSIAMProvisionStatusRole. +func (in *AWSIAMProvisionStatusRole) DeepCopy() *AWSIAMProvisionStatusRole { + if in == nil { + return nil + } + out := new(AWSIAMProvisionStatusRole) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/iam.aws.edenlab.io_awsiamprovisions.yaml b/config/crd/bases/iam.aws.edenlab.io_awsiamprovisions.yaml index 4fad224..be509b1 100644 --- a/config/crd/bases/iam.aws.edenlab.io_awsiamprovisions.yaml +++ b/config/crd/bases/iam.aws.edenlab.io_awsiamprovisions.yaml @@ -234,6 +234,114 @@ spec: type: string phase: type: string + roles: + additionalProperties: + description: AWSIAMProvisionStatusRole defines the observed state + of AWSIAMProvision's roles. + properties: + message: + type: string + phase: + type: string + status: + description: RoleStatus defines the observed state of Role + properties: + ackResourceMetadata: + description: |- + All CRs managed by ACK have a common `Status.ACKResourceMetadata` member + that is used to contain resource sync state, account ownership, + constructed ARN for the resource + properties: + arn: + description: |- + ARN is the Amazon Resource Name for the resource. This is a + globally-unique identifier and is set only by the ACK service controller + once the controller has orchestrated the creation of the resource OR + when it has verified that an "adopted" resource (a resource where the + ARN annotation was set by the Kubernetes user on the CR) exists and + matches the supplied CR's Spec field values. + https://github.com/aws/aws-controllers-k8s/issues/270 + type: string + ownerAccountID: + description: |- + OwnerAccountID is the AWS Account ID of the account that owns the + backend AWS service API resource. + type: string + region: + description: Region is the AWS region in which the resource + exists or will exist. + type: string + required: + - ownerAccountID + - region + type: object + conditions: + description: |- + All CRS managed by ACK have a common `Status.Conditions` member that + contains a collection of `ackv1alpha1.Condition` objects that describe + the various terminal states of the CR and its backend AWS service API + resource + items: + description: |- + Condition is the common struct used by all CRDs managed by ACK service + controllers to indicate terminal states of the CR and its backend AWS + service API resource + properties: + lastTransitionTime: + description: Last time the condition transitioned + from one status to another. + format: date-time + type: string + message: + description: A human readable message indicating details + about the transition. + type: string + reason: + description: The reason for the condition's last transition. + type: string + status: + description: Status of the condition, one of True, + False, Unknown. + type: string + type: + description: Type is the type of the Condition + type: string + required: + - status + - type + type: object + type: array + createDate: + description: |- + The date and time, in ISO 8601 date-time format (http://www.iso.org/iso/iso8601), + when the role was created. + format: date-time + type: string + roleID: + description: |- + The stable and unique string identifying the role. For more information about + IDs, see IAM identifiers (https://docs.aws.amazon.com/IAM/latest/UserGuide/Using_Identifiers.html) + in the IAM User Guide. + type: string + roleLastUsed: + description: |- + Contains information about the last time that an IAM role was used. This + includes the date and time and the Region in which the role was last used. + Activity is only reported for the trailing 400 days. This period can be shorter + if your Region began supporting these features within the last year. The + role might have been used more than 400 days ago. For more information, see + Regions where data is tracked (https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_access-advisor.html#access-advisor_tracking-period) + in the IAM user Guide. + properties: + lastUsedDate: + format: date-time + type: string + region: + type: string + type: object + type: object + type: object + type: object type: object type: object served: true diff --git a/internal/controller/awsiamprovision_controller.go b/internal/controller/awsiamprovision_controller.go index c1051f0..c1904cc 100644 --- a/internal/controller/awsiamprovision_controller.go +++ b/internal/controller/awsiamprovision_controller.go @@ -19,12 +19,14 @@ package controller import ( "context" "fmt" - "sigs.k8s.io/controller-runtime/pkg/log" "time" + iamctrlv1alpha1 "github.com/aws-controllers-k8s/iam-controller/apis/v1alpha1" + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" iamv1alpha1 "aws-iam-provisioner.operators.infra/api/v1alpha1" @@ -65,25 +67,42 @@ func (r *AWSIAMProvisionReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{RequeueAfter: frequency}, nil } - provisioned := false + awsIAMProvisionProvisioned := false + sourceK8sResourceStatuses := make(map[string]*iamctrlv1alpha1.RoleStatus) for name, item := range awsIAMProvision.Spec.Roles { - k8sResource, err := r.handleRole(awsIAMProvision, eksControlPlane, name, &item) + k8sResource, k8sResourceUpdated, err := r.handleRole(awsIAMProvision, eksControlPlane, name, &item) if err != nil { return ctrl.Result{}, err } if k8sResource != nil { - // If a resource has been returned, there was a change to it - provisioned = true + sourceK8sResourceStatuses[k8sResource.Name] = &k8sResource.Status + } + + if k8sResourceUpdated { + awsIAMProvisionProvisioned = true } } - if awsIAMProvision.Status.Phase != "Provisioned" || provisioned { + targetK8sResourceStatuses := make(map[string]*iamctrlv1alpha1.RoleStatus) + for name, awsIAMProvisionStatusRole := range awsIAMProvision.Status.Roles { + targetK8sResourceStatuses[name] = &awsIAMProvisionStatusRole.Status + } + + k8sResourceStatusesEqual := cmp.Equal(sourceK8sResourceStatuses, targetK8sResourceStatuses) + if k8sResourceStatusesEqual { + r.logger.Info(fmt.Sprintf("IAM Role statuses of AWSIAMProvision equal: %s", r.request.NamespacedName)) + } else { + r.logger.Info(fmt.Sprintf("IAM Role statuses of AWSIAMProvision different: %s", r.request.NamespacedName)) + } + + // Check all conditions indicating the resource or its status are actually updated + if awsIAMProvision.Status.Phase != "Provisioned" || awsIAMProvisionProvisioned || !k8sResourceStatusesEqual { // Resources have been provisioned successfully r.logger.Info(fmt.Sprintf("AWSIAMProvision provisioned: %s", r.request.NamespacedName)) - if err := r.updateCRDStatus(awsIAMProvision, "Provisioned", ""); err != nil { + if err := r.updateCRDStatus(awsIAMProvision, "Provisioned", "", sourceK8sResourceStatuses); err != nil { return ctrl.Result{}, err } } @@ -96,5 +115,6 @@ func (r *AWSIAMProvisionReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&iamv1alpha1.AWSIAMProvision{}). WithEventFilter(predicate.GenerationChangedPredicate{}). + Owns(&iamctrlv1alpha1.Role{}). // trigger the r.Reconcile whenever an Own-ed resource is created/updated/deleted Complete(r) } diff --git a/internal/controller/reconciliation_manager.go b/internal/controller/reconciliation_manager.go index 4454de5..8282cd1 100644 --- a/internal/controller/reconciliation_manager.go +++ b/internal/controller/reconciliation_manager.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "k8s.io/apimachinery/pkg/runtime" "strings" "text/template" "time" @@ -14,8 +13,10 @@ import ( ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1" "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2" ctrl "sigs.k8s.io/controller-runtime" @@ -68,14 +69,14 @@ func (rm *ReconciliationManager) getClusterResources() (*iamv1alpha1.AWSIAMProvi msg := fmt.Sprintf("AWSManagedControlPlane of %s AWSIAMProvision not found: %s", rm.request.NamespacedName, namespacedName) rm.logger.Info(msg) - if err := rm.updateCRDStatus(awsIAMProvision, "Provisioning", msg); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Provisioning", msg, nil); err != nil { return nil, nil, err } return nil, nil, nil } - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { return nil, nil, err } @@ -88,7 +89,7 @@ func (rm *ReconciliationManager) getClusterResources() (*iamv1alpha1.AWSIAMProvi msg := fmt.Sprintf("AWSManagedControlPlane of %s AWSIAMProvision not ready: %s", rm.request.NamespacedName, namespacedName) rm.logger.Info(msg) - if err := rm.updateCRDStatus(awsIAMProvision, "Provisioning", msg); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Provisioning", msg, nil); err != nil { return nil, nil, err } @@ -98,85 +99,116 @@ func (rm *ReconciliationManager) getClusterResources() (*iamv1alpha1.AWSIAMProvi return awsIAMProvision, eksControlPlane, nil } -func (rm *ReconciliationManager) handleRole(awsIAMProvision *iamv1alpha1.AWSIAMProvision, eksControlPlane *ekscontrolplanev1.AWSManagedControlPlane, name string, item *iamv1alpha1.AWSIAMProvisionRole) (*iamctrlv1alpha1.Role, error) { +func (rm *ReconciliationManager) handleRole(awsIAMProvision *iamv1alpha1.AWSIAMProvision, eksControlPlane *ekscontrolplanev1.AWSManagedControlPlane, name string, item *iamv1alpha1.AWSIAMProvisionRole) (*iamctrlv1alpha1.Role, bool, error) { k8sResource := &iamctrlv1alpha1.Role{} namespacedName := types.NamespacedName{Name: name, Namespace: rm.request.NamespacedName.Namespace} if err := rm.client.Get(*rm.context, namespacedName, k8sResource); err != nil { - if k8serrors.IsNotFound(err) { - // Create new role - if err := rm.setAssumeRolePolicyDocument(awsIAMProvision, eksControlPlane, item); err != nil { - return nil, err - } + if !k8serrors.IsNotFound(err) { + return nil, false, err + } - if err := rm.validateRolePolicyRefs(awsIAMProvision, item); err != nil { - return nil, err - } + // Create new role + if err := rm.setDefaultValues(awsIAMProvision, eksControlPlane, item); err != nil { + return nil, false, err + } - k8sResource.TypeMeta = roleMeta - k8sResource.ObjectMeta = metav1.ObjectMeta{ - Name: name, - Namespace: rm.request.NamespacedName.Namespace, - } - k8sResource.Spec = item.Spec + if err := rm.validateRolePolicyRefs(awsIAMProvision, item); err != nil { + return nil, false, err + } - // Set ownerReferences to ensure that the created resource will be deleted when the custom resource object is removed - if err := ctrl.SetControllerReference(awsIAMProvision, k8sResource, rm.scheme); err != nil { - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { - return nil, err - } + k8sResource.TypeMeta = roleMeta + k8sResource.ObjectMeta = metav1.ObjectMeta{ + Name: name, + Namespace: rm.request.NamespacedName.Namespace, + } + k8sResource.Spec = item.Spec - return nil, err + // Set ownerReferences to ensure that the created resource will be deleted when the custom resource object is removed + if err := ctrl.SetControllerReference(awsIAMProvision, k8sResource, rm.scheme); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { + return nil, false, err } - if err = rm.client.Create(*rm.context, k8sResource); err != nil { - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { - return nil, err - } + return nil, false, err + } - return nil, err + if err = rm.client.Create(*rm.context, k8sResource); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { + return nil, false, err } - rm.logger.Info(fmt.Sprintf("IAM Role of %s AWSIAMProvision created: %s", rm.request.NamespacedName, namespacedName)) - - return k8sResource, nil + return nil, false, err } - return nil, err + rm.logger.Info(fmt.Sprintf("IAM Role of %s AWSIAMProvision created: %s", rm.request.NamespacedName, namespacedName)) + + return k8sResource, true, nil } - if err := rm.setAssumeRolePolicyDocument(awsIAMProvision, eksControlPlane, item); err != nil { - return nil, err + if err := rm.setDefaultValues(awsIAMProvision, eksControlPlane, item); err != nil { + return nil, false, err } if cmp.Equal(item.Spec, k8sResource.Spec) { // No diff with existing resource, exiting without error - return nil, nil + rm.logger.Info(fmt.Sprintf("IAM Role of %s AWSIAMProvision equal: %s", rm.request.NamespacedName, namespacedName)) + + return k8sResource, false, nil } + rm.logger.Info(fmt.Sprintf("IAM Role of %s AWSIAMProvision different: %s", rm.request.NamespacedName, namespacedName)) + if err := rm.validateRolePolicyRefs(awsIAMProvision, item); err != nil { - return nil, err + return nil, false, err } // Update role with new values k8sResource.Spec = item.Spec if err := rm.client.Update(*rm.context, k8sResource); err != nil { - return nil, err + return nil, false, err } rm.logger.Info(fmt.Sprintf("IAM Role of %s AWSIAMProvision updated: %s", rm.request.NamespacedName, namespacedName)) - return k8sResource, nil + return k8sResource, true, nil } -func (rm *ReconciliationManager) updateCRDStatus(awsIAMProvision *iamv1alpha1.AWSIAMProvision, phase, message string) error { +func (rm *ReconciliationManager) updateCRDStatus(awsIAMProvision *iamv1alpha1.AWSIAMProvision, phase, message string, roleStatuses map[string]*iamctrlv1alpha1.RoleStatus) error { awsIAMProvision.Status.LastUpdatedTime = &metav1.Time{Time: time.Now()} awsIAMProvision.Status.Phase = phase awsIAMProvision.Status.Message = message + if roleStatuses != nil { + statusRoles := make(map[string]iamv1alpha1.AWSIAMProvisionStatusRole) + + for roleName, roleStatus := range roleStatuses { + statusRolePhase := "Provisioned" + statusRoleMessage := "" + + for _, condition := range roleStatus.Conditions { + // if any of roleStatus.condition.Status is not True, then the overall roleStatus.phase is considered Failed + if condition.Status != v1.ConditionTrue { + statusRolePhase = "Failed" + statusRoleMessage = *condition.Message + + break + } + } + + statusRoles[roleName] = iamv1alpha1.AWSIAMProvisionStatusRole{ + Phase: statusRolePhase, + Message: statusRoleMessage, + Status: *roleStatus, + } + } + + awsIAMProvision.Status.Roles = statusRoles + } + if err := rm.status.Update(*rm.context, awsIAMProvision); err != nil { - return errors.New(fmt.Sprintf("Unable to update status for CRD: %s", awsIAMProvision.Name)) + return errors.New(fmt.Sprintf("Unable to update status for CRD: %s, error: %s", awsIAMProvision.Name, err)) } return nil @@ -188,7 +220,7 @@ func (rm *ReconciliationManager) validateRolePolicyRefs(awsIAMProvision *iamv1al _, err := rm.getPolicy(awsIAMProvision, item, policyRef) if err != nil { - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { return err } @@ -208,7 +240,7 @@ func (rm *ReconciliationManager) getPolicy(awsIAMProvision *iamv1alpha1.AWSIAMPr err = errors.New(fmt.Sprintf("IAM Policy of %s IAM Role of %s AWSIAMProvision not found: %s", *item.Spec.Name, rm.request.NamespacedName, namespacedName)) } - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { return nil, err } @@ -218,6 +250,26 @@ func (rm *ReconciliationManager) getPolicy(awsIAMProvision *iamv1alpha1.AWSIAMPr return k8sResource, nil } +func (rm *ReconciliationManager) setDefaultValues(awsIAMProvision *iamv1alpha1.AWSIAMProvision, eksControlPlane *ekscontrolplanev1.AWSManagedControlPlane, item *iamv1alpha1.AWSIAMProvisionRole) error { + // Set default values to prevent unwanted diffs (the logic is similar to aws-iam-controller) + if item.Spec.MaxSessionDuration == nil { + defaultMaxSessionDuration := int64(3600) + item.Spec.MaxSessionDuration = &defaultMaxSessionDuration + } + + if item.Spec.Path == nil { + defaultPath := "/" + item.Spec.Path = &defaultPath + } + + // Set rendered template to detect the diff correctly + if err := rm.setAssumeRolePolicyDocument(awsIAMProvision, eksControlPlane, item); err != nil { + return err + } + + return nil +} + func (rm *ReconciliationManager) setAssumeRolePolicyDocument(awsIAMProvision *iamv1alpha1.AWSIAMProvision, eksControlPlane *ekscontrolplanev1.AWSManagedControlPlane, item *iamv1alpha1.AWSIAMProvisionRole) error { oidcProviderARN := eksControlPlane.Status.OIDCProvider.ARN _, oidcProviderName, oidcProviderARNFound := strings.Cut(oidcProviderARN, "/") @@ -226,7 +278,7 @@ func (rm *ReconciliationManager) setAssumeRolePolicyDocument(awsIAMProvision *ia namespacedName := types.NamespacedName{Name: awsIAMProvision.Spec.EksClusterName, Namespace: rm.request.NamespacedName.Namespace} err := errors.New(fmt.Sprintf("OIDC ARN of %s AWSManagedControlPlane of %s AWSIAMProvision malformed: %s", namespacedName, rm.request.NamespacedName, oidcProviderARN)) - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { return err } @@ -234,7 +286,7 @@ func (rm *ReconciliationManager) setAssumeRolePolicyDocument(awsIAMProvision *ia } if assumeRolePolicyDocument, err := rm.renderOIDCProviderTemplate(*item.Spec.AssumeRolePolicyDocument, oidcProviderARN, oidcProviderName); err != nil { - if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error()); err != nil { + if err := rm.updateCRDStatus(awsIAMProvision, "Failed", err.Error(), nil); err != nil { return err }