Console-3733: Add support for Azure Workload Identity / Federated Identity based installs in OperatorHub #13082

gallettilance · 2023-08-08T15:32:03Z

This PR adds the following:

A warning is displayed in the item modal that the cluster is in WI/FI mode if the operator claims to supports it
A warning is displayed in the item subscription page that the cluster is in WI/FI mode if the operator claims to support it
If the cluster is in WI/FI mode and the operator claims support for it the the subscription page provides configuring 3 additional fields: client ID, tenant ID, and subscription ID (we are assuming that token path will be standardized / defaulted to /var/run/secrets/openshift/serviceaccount/token and region is not a required configuration)
Default subscription to manual for installs on WI/FI mode clusters for operators that support it

Screencast.from.2023-08-08.11-52-54.webm

openshift-ci-robot · 2023-08-08T15:32:06Z

@gallettilance: This pull request references Console-3733 which is a valid jira issue.

In response to this:

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/test-infra repository.

openshift-ci-robot · 2023-08-08T15:37:32Z

@gallettilance: This pull request references Console-3733 which is a valid jira issue.

In response to this:

This PR adds the following:

A warning is displayed in the item modal that the cluster is in WI/FI mode if the operator claims to supports it

A warning is displayed in the item subscription page that the cluster is in WI/FI mode if the operator claims to support it

If the cluster is in WI/FI mode and the operator claims support for it the the subscription page provides configuring 3 additional fields: client ID, tenant ID, and subscription ID (we are assuming that token path will be standardized / defaulted to /var/run/secrets/openshift/serviceaccount/token and region is not a required configuration)

Screen capture to follow

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/test-infra repository.

gallettilance · 2023-08-08T15:38:15Z

cc @rhamilto @TheRealJon @jhadvig

TheRealJon · 2023-08-08T15:43:00Z

QE Approver
/assign @yanpzhan

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

openshift-ci-robot · 2023-08-08T15:56:59Z

@gallettilance: This pull request references Console-3733 which is a valid jira issue.

In response to this:

This PR adds the following:

A warning is displayed in the item modal that the cluster is in WI/FI mode if the operator claims to supports it

A warning is displayed in the item subscription page that the cluster is in WI/FI mode if the operator claims to support it

If the cluster is in WI/FI mode and the operator claims support for it the the subscription page provides configuring 3 additional fields: client ID, tenant ID, and subscription ID (we are assuming that token path will be standardized / defaulted to /var/run/secrets/openshift/serviceaccount/token and region is not a required configuration)

Default subscription to manual for installs on WI/FI mode clusters for operators that support it

Screencast.from.2023-08-08.11-52-54.webm

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/test-infra repository.

RickJWagner · 2023-08-08T17:55:50Z

/label px-approved

TheRealJon

/lgtm

TheRealJon · 2023-08-08T15:52:15Z

...ackages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx

+              {isAzureWIFCluster(cloudCredentials, infrastructure, authentication) &&
+                showWarn &&
+                infraFeatures?.find((i) => i === InfraFeatures[shortLivedTokenAuth]) && (


Is it possible for both the AWS and Azure warnings to be shown side by side or are they mutually exclusive? If they can, then each needs it's own "show/hide" state or else dismissing one will dismiss both.

Also, the infraFeatures check is not necessary for either of these warnings. The isAzureWIFCluster check is a prerequisite for the shortLivedTokenAuth infra feature to be included. Seen here

@TheRealJon the warnings are mutually exclusive - an operator can support both AWS and Azure but the platform will only be one or the other.

shortLivedTokenAuth is set to true when the operator supports the platform-specific short lived tokenized auth and the cluster is in that mode. So if anything it's the isAzureWIFCluster or isAWSSTSCluster that isn't required (I think) but I would like to display a warning custom to the platform (and shortLivedTokenAuth doesn't tell me the platform type) so I need a way to detect the platform type which is why isAzure... and isAWS... were added.

TheRealJon · 2023-08-08T19:17:11Z

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-items.tsx

+        currentItem.infrastructure,
+        currentItem.authentication,
+      ) &&
+      currentItem.infraFeatures?.find((i) => i === InfraFeatures[shortLivedTokenAuth])


Suggested change

currentItem.infraFeatures?.find((i) => i === InfraFeatures[shortLivedTokenAuth])

not sure I understand why - I need to know the operator claims support for the platform's specific short term auth method

TheRealJon · 2023-08-08T19:23:29Z

The comments I made are just nits. You can address them or not. Just let me know if you make changes and I'll review again.

opayne1 · 2023-08-09T14:47:22Z

/label docs-approved

gallettilance · 2023-08-16T15:36:15Z

/hold until openshift/cloud-credential-operator#587 merges

openshift-bot · 2023-11-15T01:00:22Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

rhamilto · 2023-11-15T01:18:39Z

cc: @jcaianirh

yanpzhan · 2023-12-01T13:09:23Z

@gallettilance I lauched a cluster against the pr, simulated a azure WI/FI configuration:

# oc get infrastructure cluster -o jsonpath --template '{ .status.platform }'
Azure
# oc get cloudcredential cluster -o jsonpath --template '{ .spec.credentialsMode }'
Manual
#  oc get authentication cluster -o jsonpath --template='{ .spec.serviceAccountIssuer }'
abutcher-oidc.apps.yanpz3733.qe.azure.devcluster.openshift.com

And prepared a operator with annotation: features.operators.openshift.io/token-auth-azure: 'true'
But I didn't see any warning about the cluster on this operator's item modal page and item subscription page.
I remember when I tested the pr in August, I could see warning info.
If there is something different in code recently and test steps are changed? How could I test the feature now?

gallettilance · 2023-12-05T17:37:50Z

@yanpzhan thanks for catching that! You were right there was an issue with the new infraFeatures that I've fixed and it should all be working now

rhamilto · 2023-12-05T18:13:23Z

/lgtm

yanpzhan · 2023-12-06T09:56:12Z

@gallettilance Thanks for your update, I could see the warning info on new cluster launched against the pr now.
But there is other issue, twhen click on input fields "Azure Client ID","Azure Tenant ID","Azure Subscription ID", they are set as "[object Object]" automatically, and could not be updated. If insist on clicking "Install" button, an error shows up:

Danger alert:An error occurred
Converting circular structure to JSON
--> starting at object with constructor 'HTMLInputElement'
| property '__reactFiber$abxqa1swcim' -> object with constructor 'Lu'
--- property 'stateNode' closes the circle

Could you pls help to check again?

rhamilto · 2023-12-06T13:33:20Z

Danger alert:An error occurred
Converting circular structure to JSON
--> starting at object with constructor 'HTMLInputElement'
| property '__reactFiber$abxqa1swcim' -> object with constructor 'Lu'
--- property 'stateNode' closes the circle

Related bug: https://issues.redhat.com/browse/ocpbugs-24252

gallettilance · 2023-12-06T18:07:27Z

waiting on #13416 to merge first

rhamilto · 2023-12-07T14:06:52Z

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-page.tsx

          const featuresAnnotationsObjects = [
            { key: InfraFeatures.Disconnected, value: disconnected },
            { key: InfraFeatures.FipsMode, value: fipsCompliant },
            { key: InfraFeatures.Proxy, value: proxyAware },
            { key: InfraFeatures.cnf, value: cnf },
            { key: InfraFeatures.cni, value: cni },
            { key: InfraFeatures.csi, value: csi },
+            { key: InfraFeatures.TokenAuth, value: tokenAuthSupport },


My memory on this is fuzzy as it has been awhile and this has changed a bit. featuresAnnotationsObjects really only exists in order to override legacy labels. Since tokenAuth*s are new, I would not include them here. I would preserve the existing functionality where you push the value on to infrastructureFeatures.

…stalls in OperatorHub

rhamilto · 2023-12-07T14:34:41Z

/lgtm

/hold for @gallettilance to finish testing

openshift-ci · 2023-12-07T14:40:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gallettilance, rhamilto, TheRealJon

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:

~~frontend/packages/operator-lifecycle-manager/OWNERS~~ [TheRealJon,rhamilto]

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

openshift-ci · 2023-12-07T19:16:15Z

@gallettilance: all tests passed!

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/test-infra repository. I understand the commands that are listed here.

gallettilance · 2023-12-07T22:49:25Z

/hold cancel
/label px-approved

Adding these labels since QE will not be able to add them before branch date and this needs to be in before then and extensive testing has been done prior to today.

gallettilance · 2023-12-07T22:50:14Z

/label qe-approved

openshift-ci-robot · 2023-12-07T22:50:20Z

@gallettilance: This pull request references Console-3733 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.15." or "openshift-4.15.", but it targets "openshift-4.14" instead.

In response to this:

This PR adds the following:

A warning is displayed in the item modal that the cluster is in WI/FI mode if the operator claims to supports it

A warning is displayed in the item subscription page that the cluster is in WI/FI mode if the operator claims to support it

If the cluster is in WI/FI mode and the operator claims support for it the the subscription page provides configuring 3 additional fields: client ID, tenant ID, and subscription ID (we are assuming that token path will be standardized / defaulted to /var/run/secrets/openshift/serviceaccount/token and region is not a required configuration)

Default subscription to manual for installs on WI/FI mode clusters for operators that support it

Screencast.from.2023-08-08.11-52-54.webm

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/test-infra repository.

gallettilance · 2023-12-07T22:51:51Z

/jira refresh

openshift-ci-robot · 2023-12-07T22:51:55Z

@gallettilance: This pull request references Console-3733 which is a valid jira issue.

In response to this:

/jira refresh

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/test-infra repository.

yanpzhan · 2023-12-11T07:01:17Z

Retested on cluster launched against the pr, the warning info show correctly and operator could be installed successfully.
I have a question about where the info set for "Azure Client Id", "Azure Tenant Id". "Azure Subscription Id" fields are stored after the operator is installed. Pasted the comment in jira
@gallettilance could you give me some clue about the question? Thanks very much.

rhamilto · 2024-01-29T16:04:12Z

Backported to 4.15 as a bug.

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

openshift-ci bot requested review from spadgett and TheRealJon August 8, 2023 15:33

openshift-ci bot added component/olm Related to OLM kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated labels Aug 8, 2023

openshift-ci bot assigned opayne1, RickJWagner and yanpzhan Aug 8, 2023

gallettilance force-pushed the azure-install branch from 929b6e4 to e88912d Compare August 8, 2023 15:58

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Aug 8, 2023

RickJWagner removed their assignment Aug 8, 2023

TheRealJon approved these changes Aug 8, 2023

View reviewed changes

openshift-ci bot assigned TheRealJon Aug 8, 2023

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 8, 2023

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Aug 9, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 16, 2023

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 15, 2023

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

gallettilance force-pushed the azure-install branch from 54af6c4 to f00b196 Compare December 5, 2023 17:10

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2023

gallettilance force-pushed the azure-install branch from f00b196 to 37248d3 Compare December 6, 2023 18:03

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2023

gallettilance force-pushed the azure-install branch from 37248d3 to 4393c3d Compare December 7, 2023 13:54

rhamilto reviewed Dec 7, 2023

View reviewed changes

Add support for Azure Workload Identity / Federated Identity based in…

fef7451

…stalls in OperatorHub

gallettilance force-pushed the azure-install branch from 4393c3d to fef7451 Compare December 7, 2023 14:29

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2023

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Dec 7, 2023

openshift-merge-bot bot merged commit ee66c50 into openshift:master Dec 8, 2023
6 checks passed

adellape mentioned this pull request Feb 22, 2024

OSDOCS#8868,8869: Add Azure Identity CCO/OLM docs openshift/openshift-docs#70222

Merged

1 task

Console-3733: Add support for Azure Workload Identity / Federated Identity based installs in OperatorHub #13082

Console-3733: Add support for Azure Workload Identity / Federated Identity based installs in OperatorHub #13082

Conversation

gallettilance commented Aug 8, 2023 • edited

openshift-ci-robot commented Aug 8, 2023 • edited by openshift-ci bot

openshift-ci-robot commented Aug 8, 2023 • edited by openshift-ci bot

gallettilance commented Aug 8, 2023

TheRealJon commented Aug 8, 2023

openshift-ci-robot commented Aug 8, 2023 • edited by openshift-ci bot

RickJWagner commented Aug 8, 2023

TheRealJon left a comment

Choose a reason for hiding this comment

TheRealJon Aug 8, 2023

Choose a reason for hiding this comment

TheRealJon Aug 8, 2023

Choose a reason for hiding this comment

gallettilance Aug 9, 2023

Choose a reason for hiding this comment

gallettilance Aug 9, 2023

Choose a reason for hiding this comment

TheRealJon Aug 8, 2023

Choose a reason for hiding this comment

gallettilance Aug 9, 2023

Choose a reason for hiding this comment

TheRealJon commented Aug 8, 2023

opayne1 commented Aug 9, 2023

gallettilance commented Aug 16, 2023

openshift-bot commented Nov 15, 2023

rhamilto commented Nov 15, 2023

yanpzhan commented Dec 1, 2023 • edited

gallettilance commented Dec 5, 2023

rhamilto commented Dec 5, 2023

yanpzhan commented Dec 6, 2023 • edited

rhamilto commented Dec 6, 2023

gallettilance commented Dec 6, 2023

rhamilto Dec 7, 2023

Choose a reason for hiding this comment

rhamilto commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

openshift-ci bot commented Dec 7, 2023

gallettilance commented Dec 7, 2023

gallettilance commented Dec 7, 2023

openshift-ci-robot commented Dec 7, 2023 • edited by openshift-ci bot

gallettilance commented Dec 7, 2023

openshift-ci-robot commented Dec 7, 2023 • edited by openshift-ci bot

yanpzhan commented Dec 11, 2023

rhamilto commented Jan 29, 2024

gallettilance commented Aug 8, 2023 •

edited

openshift-ci-robot commented Aug 8, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Aug 8, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Aug 8, 2023 •

edited by openshift-ci bot

yanpzhan commented Dec 1, 2023 •

edited

yanpzhan commented Dec 6, 2023 •

edited

openshift-ci-robot commented Dec 7, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Dec 7, 2023 •

edited by openshift-ci bot