Address PR review comments

Co-authored-by: Richard Case <[email protected]>

Make OIDC IAM permissions optional

rename feature to distinguish from managed mode

Rework OIDC reconcile to work mgmt side only

Fixes and tidy up

Add OIDC provider flavor

use keyIDFromPublicKey from k8s code

Gate OIDC reconcile behind feature flag.

Revert unneeded awsmachine_controller test changes

fix clusterawsadm template tests

Fix lints & tests

bump pod ID webhook to latest (v0.6.6)

Author: sl1pm4t

api/v1beta2/awscluster_spec.go

@@ -11,10 +11,10 @@ func (s *AWSClusterSpec) Validate() []*field.Error {
 	var errs field.ErrorList
 
 	// Check the feature gate is enabled for OIDC Provider.
-	if s.AssociateOIDCProvider && !feature.Gates.Enabled(feature.OIDCProviderSupport) {
+	if s.AssociateOIDCProvider && !feature.Gates.Enabled(feature.OIDCProviderUnmanagedClusters) {
 		errs = append(errs,
 			field.Forbidden(field.NewPath("spec", "associateOIDCProvider"),
-				"can be enabled only if the OIDCProviderSupport feature gate is enabled"),
+				"can be enabled only if the OIDCProviderUnmanagedClusters feature gate is enabled"),
 		)
 		return errs
 	}

api/v1beta2/awscluster_types.go

@@ -108,7 +108,7 @@ type AWSClusterSpec struct {
 	// S3Bucket contains options to configure a supporting S3 bucket for this
 	// cluster - Used for nodes requiring Ignition (https://coreos.github.io/ignition/) for bootstrapping (requires
 	// BootstrapFormatIgnition feature flag to be enabled) and/or for storing OIDC endpoint certificates for use
-	// with IRSA (requires OIDCProviderSupport feature flag to be enabled).
+	// with IRSA (requires OIDCProviderUnmanagedClusters feature flag to be enabled).
 	// +optional
 	S3Bucket *S3Bucket `json:"s3Bucket,omitempty"`
 
@@ -290,7 +290,7 @@ type AWSClusterStatus struct {
 
 	// OIDCProvider holds the status of the identity provider for this cluster
 	// +optional
-	OIDCProvider OIDCProviderStatus `json:"oidcProvider,omitempty"`
+	OIDCProvider *OIDCProviderStatus `json:"oidcProvider,omitempty"`
 }
 
 // S3Bucket defines a supporting S3 bucket for the cluster, currently can be optionally used for Ignition.

api/v1beta2/s3bucket.go

@@ -37,10 +37,11 @@ func (b *S3Bucket) Validate() []*field.Error {
 		errs = append(errs, field.Required(field.NewPath("spec", "s3Bucket", "name"), "can't be empty"))
 	}
 
-	// Feature gate is not enabled but ignition is enabled then send a forbidden error.
-	if !feature.Gates.Enabled(feature.BootstrapFormatIgnition) && !feature.Gates.Enabled(feature.OIDCProviderSupport) {
+	// Either the BootstrapFormatIgnition or OIDCProviderUnmanagedClusters feature gate must be enabled.
+	// Otherwise send a forbidden error.
+	if !feature.Gates.Enabled(feature.BootstrapFormatIgnition) && !feature.Gates.Enabled(feature.OIDCProviderUnmanagedClusters) {
 		errs = append(errs, field.Forbidden(field.NewPath("spec", "s3Bucket"),
-			"can be set only if the BootstrapFormatIgnition or OIDCProviderSupport feature gate is enabled"))
+			"can be set only if the BootstrapFormatIgnition or OIDCProviderUnmanagedClusters feature gate is enabled"))
 	}
 
 	if b.PresignedURLDuration == nil {

api/v1beta2/zz_generated.deepcopy.go

@@ -106,6 +106,11 @@ func SetDefaults_AWSIAMConfigurationSpec(obj *AWSIAMConfigurationSpec) { //nolin
 	if obj.S3Buckets.NamePrefix == "" {
 		obj.S3Buckets.NamePrefix = DefaultS3BucketPrefix
 	}
+	if obj.IAMIdentityProviders == nil {
+		obj.IAMIdentityProviders = &IAMIdentityProviders{
+			Enable: false,
+		}
+	}
 }
 
 // SetDefaults_AWSIAMConfiguration is used by defaulter-gen.

cmd/clusterawsadm/api/bootstrap/v1alpha1/zz_generated.conversion.go

@@ -178,6 +178,13 @@ type S3Buckets struct {
 	NamePrefix string `json:"namePrefix"`
 }
 
+// IAMIdentityProviders controls the configuration of the AWS IAM role for IAM
+// Identity providers, which can be created to support IRSA for unmanaged clusters.
+type IAMIdentityProviders struct {
+	// Enable controls whether permissions are granted to manage IAM Identity Providers
+	Enable bool `json:"enable,omitempty"`
+}
+
 // AWSIAMConfigurationSpec defines the specification of the AWSIAMConfiguration.
 type AWSIAMConfigurationSpec struct {
 	// NamePrefix will be prepended to every AWS IAM role, user and policy created by clusterawsadm. Defaults to "".
@@ -235,6 +242,11 @@ type AWSIAMConfigurationSpec struct {
 
 	// AllowAssumeRole enables the sts:AssumeRole permission within the CAPA policies
 	AllowAssumeRole bool `json:"allowAssumeRole,omitempty"`
+
+	// IAMIdentityProviders, when enabled, will add controller nodes permissions to
+	// create IAM Identity Providers for kubeadm workload clusters.
+	// +optional
+	IAMIdentityProviders *IAMIdentityProviders `json:"iamIdentityProviders,omitempty"`
 }
 
 // GetObjectKind returns the AAWSIAMConfiguration's TypeMeta.

cmd/clusterawsadm/api/bootstrap/v1beta1/defaults.go

@@ -185,11 +185,6 @@ func (t Template) ControllersPolicy() *iamv1.PolicyDocument {
 				"ec2:DeleteLaunchTemplateVersions",
 				"ec2:DescribeKeyPairs",
 				"ec2:ModifyInstanceMetadataOptions",
-				"iam:CreateOpenIDConnectProvider",
-				"iam:DeleteOpenIDConnectProvider",
-				"iam:ListOpenIDConnectProviders",
-				"iam:GetOpenIDConnectProvider",
-				"iam:TagOpenIDConnectProvider",
 			},
 		},
 		{
@@ -301,6 +296,16 @@ func (t Template) ControllersPolicy() *iamv1.PolicyDocument {
 				"s3:DeleteObject",
 				"s3:PutBucketPolicy",
 				"s3:PutBucketTagging",
+			},
+		})
+	}
+	if t.Spec.IAMIdentityProviders.Enable {
+		statement = append(statement, iamv1.StatementEntry{
+			Effect: iamv1.EffectAllow,
+			Resource: iamv1.Resources{
+				fmt.Sprintf("arn:*:s3:::%s*", t.Spec.S3Buckets.NamePrefix),
+			},
+			Action: iamv1.Actions{
 				"s3:PutBucketOwnershipControls",
 				"s3:PutObjectAcl",
 				"s3:PutBucketPublicAccessBlock",
@@ -328,6 +333,19 @@ func (t Template) ControllersPolicy() *iamv1.PolicyDocument {
 			},
 		})
 	}
+	if t.Spec.IAMIdentityProviders.Enable {
+		statement = append(statement, iamv1.StatementEntry{
+			Effect:   iamv1.EffectAllow,
+			Resource: iamv1.Resources{iamv1.Any},
+			Action: iamv1.Actions{
+				"iam:CreateOpenIDConnectProvider",
+				"iam:DeleteOpenIDConnectProvider",
+				"iam:ListOpenIDConnectProviders",
+				"iam:GetOpenIDConnectProvider",
+				"iam:TagOpenIDConnectProvider",
+			},
+		})
+	}
 
 	return &iamv1.PolicyDocument{
 		Version:   iamv1.CurrentVersion,

cmd/clusterawsadm/api/bootstrap/v1beta1/types.go

@@ -245,11 +245,6 @@ Resources:
           - ec2:DeleteLaunchTemplateVersions
           - ec2:DescribeKeyPairs
           - ec2:ModifyInstanceMetadataOptions
-          - iam:CreateOpenIDConnectProvider
-          - iam:DeleteOpenIDConnectProvider
-          - iam:ListOpenIDConnectProviders
-          - iam:GetOpenIDConnectProvider
-          - iam:TagOpenIDConnectProvider
           Effect: Allow
           Resource:
           - '*'