OCPBUGS-42434: Enable Managed Identity w/ Certificates in HyperShift Control Plane Components #5160

bryan-cox · 2024-11-20T01:27:43Z

What this PR does / why we need it:
This PR enables components in the hosted control plane to authenticate with Azure Cloud through client certificate. These components include:

KMS
CPO
CAPZ
Cloud Provider

Which issue(s) this PR fixes:
Fixes OCPBUGS-42434

Checklist

Subject and description added to both, commit and PR.
Relevant issues have been referenced.
This change includes docs.
This change includes unit tests.

openshift-ci · 2024-11-20T01:27:47Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-11-20T01:28:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [bryan-cox]

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

netlify · 2024-11-20T01:29:23Z

✅ Deploy Preview for hypershift-docs ready!

Name	Link
🔨 Latest commit	`ef8852b`
🔍 Latest deploy log	https://app.netlify.com/sites/hypershift-docs/deploys/673dd254c493be0008a659e4
😎 Deploy Preview	https://deploy-preview-5160--hypershift-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Reconcile the SecretProviderClass for the ingress operator for ARO HCP deployments. The SecretProviderClass is used by the Secrets Store CSI driver to mount a certificate to a volume in the ingress pod deployment. Signed-off-by: Bryan Cox <brcox@redhat.com>

Signed-off-by: Bryan Cox <brcox@redhat.com>

Reconcile the Azure credentials information secret to a fixed secret name, azure-credential-information, in the hosted control plane namespace. Signed-off-by: Bryan Cox <brcox@redhat.com>

This commit authenticates Azure KMS with certificate authentication in order to communicate with Azure Cloud API. The certificate is stored in an Azure key vault and mounted into the KAS pod through a Secrets Store CSI driver SecretProviderClass. Signed-off-by: Bryan Cox <brcox@redhat.com>

Reconcile the SecretProviderClass for the control plane operator for ARO HCP deployments. The SecretProviderClass is used by the Secrets Store CSI driver to mount a certificate to a volume in the control plane operator pod deployment. Signed-off-by: Bryan Cox <brcox@redhat.com>

Update the go.mod to include the specific commit that includes the changes to allow service principal with certificate to use a certificate . Signed-off-by: Bryan Cox <brcox@redhat.com>

This commit authenticates CAPZ with certificate authentication in order to communicate with Azure Cloud API. The certificate is stored in an Azure key vault and mounted into the capi-provider pod through a Secrets Store CSI driver SecretProviderClass. Signed-off-by: Bryan Cox <brcox@redhat.com>

openshift-ci-robot · 2024-11-20T12:17:56Z

@bryan-cox: This pull request references Jira Issue OCPBUGS-42434, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @fxierh

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

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 openshift-eng/jira-lifecycle-plugin repository.

bryan-cox · 2024-11-20T12:42:55Z

/retest

This commit changes the authentication from client secret to client certificate as well as reconciling the SecretProviderClass for cloud provider for ARO HCP deployments. The SecretProviderClass is used by the Secrets Store CSI driver to mount a certificate to a volume in the azure-cloud-controller-manager pod's deployment. Signed-off-by: Bryan Cox <brcox@redhat.com>

bryan-cox · 2024-11-20T15:29:35Z

/test e2e-aks

bryan-cox · 2024-11-20T15:29:49Z

/test e2e-aws

Patryk-Stefanski · 2024-11-20T15:40:36Z

control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go

+			return fmt.Errorf("failed to get Azure credentials secret: %w", err)
+		}
+
+		tenantID := strings.TrimSpace(string(credentialsSecret.Data["AZURE_TENANT_ID"]))


Do we need to trim spaces here?

Patryk-Stefanski · 2024-11-20T15:41:02Z

control-plane-operator/controllers/hostedcontrolplane/cloud/azure/reconcile.go

@@ -2,6 +2,7 @@ package azure

 import (
 	"fmt"
+	"github.com/openshift/hypershift/support/azureutil"


Sort imports

Patryk-Stefanski · 2024-11-20T16:05:23Z

...ol-plane-operator/controllers/hostedcontrolplane/v2/cloud_controller_manager/azure/config.go

@@ -3,6 +3,9 @@ package azure
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/secretproviderclass"


sort imports

bryan-cox · 2024-11-20T16:24:51Z

/test e2e-aws-4-17

Signed-off-by: Bryan Cox <brcox@redhat.com>

bryan-cox · 2024-11-20T19:59:39Z

/test unit

bryan-cox · 2024-11-20T20:13:20Z

/retest

openshift-ci · 2024-11-20T21:54:34Z

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

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aks	`f5ff648`	link	true	`/test e2e-aks`
ci/prow/e2e-aws-4-17	`f5ff648`	link	true	`/test e2e-aws-4-17`

Full PR test history. Your PR dashboard.

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.

muraee · 2024-11-21T11:19:27Z

...lane-operator/controllers/hostedcontrolplane/v2/cloud_controller_manager/azure/deployment.go

@@ -20,5 +22,12 @@ func adaptDeployment(cpContext component.ControlPlaneContext, deployment *appsv1
 			fmt.Sprintf("--cluster-name=%s", cpContext.HCP.Spec.InfraID),
 		)
 	})
+	deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(deployment.Spec.Template.Spec.Containers[0].VolumeMounts,


please move this inside util.UpdateContainer function above and use the passed in container instead of deployment.Spec.Template.Spec.Containers[0]

muraee · 2024-11-21T11:20:06Z

...ol-plane-operator/controllers/hostedcontrolplane/v2/cloud_controller_manager/azure/config.go

@@ -49,14 +51,17 @@ func adaptConfigSecret(cpContext component.ControlPlaneContext, secret *corev1.S
 	return nil
 }

+func adaptSecretProvider(cpContext component.ControlPlaneContext, secretProvider *secretsstorev1.SecretProviderClass) error {
+	secretProvider = manifests.ManagedAzureSecretProviderClass(secretProvider.Name, cpContext.HCP.Namespace)


Suggested change

secretProvider = manifests.ManagedAzureSecretProviderClass(secretProvider.Name, cpContext.HCP.Namespace)

This line is not needed

openshift-merge-robot · 2024-11-27T10:38:28Z

PR needs rebase.

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.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2024

openshift-ci bot added do-not-merge/needs-area area/api Indicates the PR includes changes for the API labels Nov 20, 2024

bryan-cox force-pushed the OCPBUGS-42434-combined branch 4 times, most recently from 7cdb04d to 3ce0090 Compare November 20, 2024 03:05

bryan-cox marked this pull request as ready for review November 20, 2024 03:05

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2024

openshift-ci bot requested review from enxebre and hasueki November 20, 2024 03:06

bryan-cox added 7 commits November 20, 2024 07:08

Add DNS RG to Ingress SP's contributor role scope

1b3ec16

Signed-off-by: Bryan Cox <brcox@redhat.com>

Reconcile Azure Creds to fixed secret in HCP NS

222444b

Reconcile the Azure credentials information secret to a fixed secret name, azure-credential-information, in the hosted control plane namespace. Signed-off-by: Bryan Cox <brcox@redhat.com>

Update go.mod to include cert changes for CAPZ

040d1b3

Update the go.mod to include the specific commit that includes the changes to allow service principal with certificate to use a certificate . Signed-off-by: Bryan Cox <brcox@redhat.com>

bryan-cox force-pushed the OCPBUGS-42434-combined branch from 3ce0090 to ef8852b Compare November 20, 2024 12:13

bryan-cox changed the title ~~Ocpbugs 42434 combined~~ OCPBUGS-42434: Enable Managed Identity w/ Certificates in HyperShift Control Plane Components Nov 20, 2024

openshift-ci-robot added the jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. label Nov 20, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 20, 2024

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Nov 20, 2024

openshift-ci bot requested a review from fxierh November 20, 2024 12:17

bryan-cox force-pushed the OCPBUGS-42434-combined branch from ef8852b to 23241af Compare November 20, 2024 13:10

Patryk-Stefanski reviewed Nov 20, 2024

View reviewed changes

WIP - add rbac for cpo to update secretprovider

f5ff648

Signed-off-by: Bryan Cox <brcox@redhat.com>

openshift-ci-robot mentioned this pull request Nov 21, 2024

OCPBUGS-42434: Add the ability to auth via certs without storing them in etcd secret openshift/cluster-api-provider-azure#323

Merged

muraee reviewed Nov 21, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-42434: Enable Managed Identity w/ Certificates in HyperShift Control Plane Components #5160

OCPBUGS-42434: Enable Managed Identity w/ Certificates in HyperShift Control Plane Components #5160

bryan-cox commented Nov 20, 2024 •

edited

Loading

openshift-ci bot commented Nov 20, 2024

openshift-ci bot commented Nov 20, 2024

netlify bot commented Nov 20, 2024 •

edited

Loading

openshift-ci-robot commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

Patryk-Stefanski Nov 20, 2024

Patryk-Stefanski Nov 20, 2024

Patryk-Stefanski Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

openshift-ci bot commented Nov 20, 2024

muraee Nov 21, 2024

muraee Nov 21, 2024

openshift-merge-robot commented Nov 27, 2024

OCPBUGS-42434: Enable Managed Identity w/ Certificates in HyperShift Control Plane Components #5160

Are you sure you want to change the base?

OCPBUGS-42434: Enable Managed Identity w/ Certificates in HyperShift Control Plane Components #5160

Conversation

bryan-cox commented Nov 20, 2024 • edited Loading

openshift-ci bot commented Nov 20, 2024

openshift-ci bot commented Nov 20, 2024

netlify bot commented Nov 20, 2024 • edited Loading

✅ Deploy Preview for hypershift-docs ready!

openshift-ci-robot commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

Patryk-Stefanski Nov 20, 2024

Choose a reason for hiding this comment

Patryk-Stefanski Nov 20, 2024

Choose a reason for hiding this comment

Patryk-Stefanski Nov 20, 2024

Choose a reason for hiding this comment

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

bryan-cox commented Nov 20, 2024

openshift-ci bot commented Nov 20, 2024

muraee Nov 21, 2024

Choose a reason for hiding this comment

muraee Nov 21, 2024

Choose a reason for hiding this comment

openshift-merge-robot commented Nov 27, 2024

bryan-cox commented Nov 20, 2024 •

edited

Loading

netlify bot commented Nov 20, 2024 •

edited

Loading