✨ WIP: Add the FIPS field to the RosaControlPlane CR, and add tests. by tinaafitz · Pull Request #5873 · kubernetes-sigs/cluster-api-provider-aws

tinaafitz · 2026-02-23T19:45:59Z

/kind feature

The acceptance criteria states that the FIPS state can only be selected at cluster install time

k8s-ci-robot · 2026-02-23T19:46:01Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2026-02-23T19:46:08Z

Hi @tinaafitz. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go

tinaafitz · 2026-02-24T00:02:41Z

Hi @serngawy, Changes made. Please review.

k8s-ci-robot · 2026-02-24T14:23:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign serngawy for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go

serngawy · 2026-02-24T16:06:19Z

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go

+	FIPSEnabled FIPS = "Enabled"
+
+	// FIPSDisabled disables FIPS mode (default)
+	FIPSDisabled FIPS = "Disabled"


Suggested change

FIPSDisabled FIPS = "Disabled"

Disabled FIPS = "Disabled"

serngawy · 2026-02-24T16:06:26Z

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go

 )

+// FIPS specifies the FIPS mode for the ROSA Control Plane.
+type FIPS string


can we generalize this enum, it is similar to autoNode. lets make it as state so in future can be re-used with any boolean kind of fields

Suggested change

type FIPS string

type mode/state string

Added State.

serngawy · 2026-02-24T16:07:19Z

controlplane/rosa/controllers/rosacontrolplane_controller.go

 		AWSCreator:                   creator,
 		AuditLogRoleARN:              ptr.To(controlPlaneSpec.AuditLogRoleARN),
 		ExternalAuthProvidersEnabled: controlPlaneSpec.EnableExternalAuthProviders,
+		FIPS:                         controlPlaneSpec.FIPS == "Enabled",


compare with the enum type created above

serngawy · 2026-02-24T16:09:56Z

controlplane/rosa/controllers/rosacontrolplane_controller_test.go

+	})
+
+	// Test case 3: Zero value FIPS (should be false)
+	t.Run("FIPS Zero Value", func(t *testing.T) {


I think this similar to the above test-case , why duplication ?

Hi @serngawy, this test case differs in that the controlPlaneSpec FIPSfield is not provided. The first test sets FIPS to Enabled, the second to Disabled, and the third doesn't set it.

Updated TestBuildOCMClusterSpec test cases to pass nil for roleConfig and rosaNet parameters to match the current function signature.

serngawy · 2026-02-24T19:34:33Z

/ok-to-test

tinaafitz · 2026-02-26T19:27:34Z

The CAPA code only enables etcd encryption if a KMS ARN is provided:
EtcdEncryption: controlPlaneSpec.EtcdEncryptionKMSARN != ""

OCM requires etcd encryption to be enabled when FIPS is enabled.

The rosa CLI (v1.2.59) requires a KMS ARN for etcd encryption on ROSA HCP clusters.
Testing rosa create cluster --fips --etcd-encryption:

When you use
--etcd-encryption, it prompts for --etcd-encryption-kms-arn.

From rosa help:

 --etcd-encryption   Add etcd encryption. By default etcd data is encrypted at rest. This option configures 
 etcd encryption on top of existing storage encryption.

The help text says:

 1. --etcd-encryption is a boolean flag that "adds etcd encryption"
 2. --etcd-encryption-kms-arn is OPTIONAL and "if set it will override etcd-encryption flag to true"

I added the validation below to ensure the KMS ARN is set when FIPS is enabled.


 Validation Error:
 {
   "type": "ROSAControlPlaneValid",
   "status": "False",
   "reason": "InvalidConfiguration",
   "severity": "Error",
   "message": "etcdEncryptionKMSARN is required when fips is Enabled. Create a KMS key, tag it with 'red-hat:true', and provide the ARN."
 }

Updated Documentation

tinaafitz · 2026-02-26T19:30:42Z

@serngawy, Please review.

serngawy · 2026-02-26T19:34:44Z

controlplane/rosa/controllers/rosacontrolplane_controller.go

 		}
 	}

+	// Validate FIPS requirements


Based on the discussion this is not true, enable ectd-encryption and fips doesn't requires kmsARN

tinaafitz · 2026-02-26T20:38:00Z

/retest

Fixed Gomega instance scoping in TestBuildOCMClusterSpec to prevent "subtest may have called FailNow on a parent test" error. Moved NewWithT(t) from function level into each subtest so each has its own Gomega instance bound to the correct *testing.T. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Remove the validation that requires etcdEncryptionKMSARN when FIPS is enabled. The validation contradicts: 1. Rosa CLI documentation - etcd-encryption-kms-arn is OPTIONAL 2. Developer comment stating "fips doesn't requires kmsARN" 3. Implementation code which already enables etcd encryption when FIPS is enabled The implementation at line 1198 correctly handles FIPS: - Enables etcd encryption when FIPS is enabled OR KMS ARN is provided - Passes KMS ARN to OCM if provided (optional enhancement) This allows users to enable FIPS with default etcd encryption without requiring them to create and manage a custom KMS key. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

k8s-ci-robot · 2026-02-27T17:07:17Z

@tinaafitz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-aws-test	`754aeff`	link	true	`/test pull-cluster-api-provider-aws-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

serngawy · 2026-02-27T20:04:00Z

controlplane/rosa/controllers/rosacontrolplane_controller.go

 	}

+	// Validate FIPS requirements
+	if rosaControlPlane.Spec.FIPS == rosacontrolplanev1.Enabled && rosaControlPlane.Spec.EtcdEncryptionKMSARN == "" {


please move this check to the webhook validation

serngawy · 2026-02-27T20:05:02Z

controlplane/rosa/controllers/rosacontrolplane_controller.go

+		RoleARN:          installerRoleARN,
+		SupportRoleARN:   supportRoleARN,
+		WorkerRoleARN:    workerRoleARN,
+		OperatorIAMRoles: operatorRoles,
+		OidcConfigId:     oidcConfigID,


no need to change this

serngawy · 2026-02-27T20:06:01Z

controlplane/rosa/controllers/rosacontrolplane_controller.go

 		}
 	}

+	// Determine role ARNs source - either from RoleConfig or manual configuration


not sure why you adding this part ?

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 23, 2026

k8s-ci-robot requested review from damdo and serngawy February 23, 2026 19:46

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-priority labels Feb 23, 2026

tinaafitz force-pushed the add_fips_support branch from b549921 to 6ed9935 Compare February 23, 2026 19:55

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/contains-merge-commits labels Feb 23, 2026

Added the FIPS field to the RosaControlPlane CR, and added tests.

62769f2

tinaafitz force-pushed the add_fips_support branch from 6ed9935 to 62769f2 Compare February 23, 2026 20:15

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2026

serngawy suggested changes Feb 23, 2026

View reviewed changes

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go Outdated Show resolved Hide resolved

controlplane/rosa/api/v1beta2/rosacontrolplane_types.go Show resolved Hide resolved

Set FIPS to Disabled by default.

8dec81b

Update documentation.

bda056c

serngawy suggested changes Feb 24, 2026

View reviewed changes

tinaafitz added 2 commits February 24, 2026 11:49

Added State for Disabled/Enabled.

f2dc127

fix: update test calls to match buildOCMClusterSpec signature

b22c01d

Updated TestBuildOCMClusterSpec test cases to pass nil for roleConfig and rosaNet parameters to match the current function signature.

tinaafitz force-pushed the add_fips_support branch from e4e62e6 to b22c01d Compare February 24, 2026 17:20

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Feb 24, 2026

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 24, 2026

Add FIPS requirement validation.

053862f

serngawy reviewed Feb 26, 2026

View reviewed changes

tinaafitz added 2 commits February 26, 2026 15:14

Fixed tests.

5c963d1

Fixed test failures.

3be053c

tinaafitz and others added 4 commits February 26, 2026 15:52

Run make generate to update CRD for FIPS field.

fcd48c3

Added validation.

754aeff

serngawy suggested changes Feb 27, 2026

View reviewed changes

Conversation

tinaafitz commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Feb 23, 2026

Uh oh!

k8s-ci-robot commented Feb 23, 2026

Uh oh!

Uh oh!

Uh oh!

tinaafitz commented Feb 24, 2026

Uh oh!

k8s-ci-robot commented Feb 24, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serngawy commented Feb 24, 2026

Uh oh!

tinaafitz commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tinaafitz commented Feb 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tinaafitz commented Feb 26, 2026

Uh oh!

k8s-ci-robot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tinaafitz commented Feb 23, 2026 •

edited

Loading

tinaafitz commented Feb 26, 2026 •

edited

Loading

k8s-ci-robot commented Feb 27, 2026 •

edited

Loading