Skip to content

CCO-324: add support for workload identity #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
openshift-merge-robot merged 6 commits into from
Jun 22, 2023
Merged

CCO-324: add support for workload identity #78

openshift-merge-robot merged 6 commits into from
Jun 22, 2023

Conversation

@RomanBednar
Copy link
Contributor

@RomanBednar RomanBednar commented Apr 21, 2023 •
edited
Loading

Driver controller deployment should load workload identity values to injector env to enable workload identity authentication.

The following keys need to be flagged as optional because their presence now depends on authentication method used:

  • azure_client_secret
  • azure_tenant_id
  • azure_federated_token_file

Depends on: openshift/cluster-cloud-controller-manager-operator#245

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 21, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Apr 21, 2023 •
edited by openshift-ci bot
Loading

@RomanBednar: This pull request references CCO-324 which is a valid jira issue.

Details

In response to this:

Driver controller deployment should load workload identity values to injector env to enable workload identity authentication.

Depends on: openshift/cluster-cloud-controller-manager-operator#245

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 openshift-ci bot requested review from bertinatto and tsmetana April 21, 2023 14:01
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from 644df85 to 7e237d0 Compare May 9, 2023 13:20
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 9, 2023 •
edited by openshift-ci bot
Loading

@RomanBednar: This pull request references CCO-324 which is a valid jira issue.

Details

In response to this:

Driver controller deployment should load workload identity values to injector env to enable workload identity authentication.

The following keys need to be flagged as optional because their presence now depends on authentication method used:

  • azure_client_secret
  • azure_tenant_id
  • azure_federated_token_file

Depends on: openshift/cluster-cloud-controller-manager-operator#245

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.

@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from 7e237d0 to 6bd9646 Compare May 17, 2023 14:08
@RomanBednar RomanBednar changed the title CCO-324: add support for workload identity WIP: CCO-324: add support for workload identity May 17, 2023
@openshift-ci 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 May 17, 2023
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from 6bd9646 to 3c4cf95 Compare May 22, 2023 12:41
jsafrane
jsafrane reviewed May 22, 2023
View reviewed changes
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch 2 times, most recently from 1feebbf to ae1b3ce Compare May 24, 2023 10:56
jsafrane
jsafrane reviewed May 24, 2023
View reviewed changes
jsafrane
jsafrane reviewed May 24, 2023
View reviewed changes
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch 4 times, most recently from c845c15 to f4cde60 Compare May 26, 2023 09:57
@RomanBednar RomanBednar mentioned this pull request May 26, 2023
Merged
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from f4cde60 to ff11d3b Compare May 26, 2023 14:49
@RomanBednar
Copy link
Contributor Author

RomanBednar commented May 26, 2023

We should get version from CSO: openshift/cluster-storage-operator#372

@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from ff11d3b to 8d8b232 Compare June 1, 2023 10:32
@RomanBednar
Copy link
Contributor Author

RomanBednar commented Jun 2, 2023 •
edited
Loading

e2e-azure-csi is failing due to missing permissions in SCA:

"Account with ID 2DUeKzzTD9ngfsQ6YgkzdJn1jA4 denied access to perform create on Certificate with HTTP call POST /api/accounts_mgmt/v1/certificates"

This is being worked on but currently blocks this PR: https://redhat-internal.slack.com/archives/C014N2VLTQE/p1683198922055559

@RomanBednar
Copy link
Contributor Author

RomanBednar commented Jun 2, 2023 •
edited
Loading

e2e-azure is failing due to RBAC:

Failed to watch *v1.ClusterVersion: failed to list *v1.ClusterVersion: clusterversions.config.openshift.io is forbidden: User "system:serviceaccount:openshift-cluster-csi-drivers:azure-disk-csi-driver-operator" cannot list resource "clusterversions" in API group "config.openshift.io" at the cluster scope

This will be fixed by a PR in CSO which will have to merge prior to this one: openshift/cluster-storage-operator#372

This was referenced Jun 2, 2023
Merged
Closed
@RomanBednar RomanBednar changed the title WIP: CCO-324: add support for workload identity CCO-324: add support for workload identity Jun 2, 2023
@openshift-ci 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 Jun 2, 2023
@RomanBednar
Copy link
Contributor Author

RomanBednar commented Jun 5, 2023

/retest-required

@RomanBednar
Copy link
Contributor Author

RomanBednar commented Jun 6, 2023

/retest e2e-azure-csi

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 6, 2023

@RomanBednar: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test e2e-azure
  • /test e2e-azure-csi
  • /test images
  • /test unit
  • /test verify-deps

The following commands are available to trigger optional jobs:

  • /test e2e-azurestack-csi

Use /test all to run all jobs.

Details

In response to this:

/retest e2e-azure-csi

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.

@RomanBednar
Copy link
Contributor Author

RomanBednar commented Jun 7, 2023

/retest e2e-azure-csi

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 7, 2023

@RomanBednar: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test e2e-azure
  • /test e2e-azure-csi
  • /test images
  • /test unit
  • /test verify-deps

The following commands are available to trigger optional jobs:

  • /test e2e-azurestack-csi

Use /test all to run all jobs.

Details

In response to this:

/retest e2e-azure-csi

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.

@RomanBednar RomanBednar force-pushed the injector-workload-identity branch 2 times, most recently from 5f8bf89 to 7f0cf97 Compare June 13, 2023 13:31
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2023
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from 7f0cf97 to 30fddc5 Compare June 13, 2023 13:31
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2023
@RomanBednar
make azure_client_secret optional
c0c93b3 
Due to addition of Azure workload identity feature, ccoctl will no
longer provide azure_client_secret in all configurations. If the
feature is enabled no client secret will be set.
@RomanBednar
load optional tenant id and token path from azure credentials secret
dbb2838 
If Azure workload identity is enabled two new secrets will be provided
by ccoctl for tenant id and path to federated token file. Those have to
be optional because if the feature is disabled those values will not
be set.
@RomanBednar
make asset functions more dynamic
a76c32c 
We need to make asset functions more dynamic. Currently we replace only
one value but in next patches we will need to also set arguments for
azure credential injector. This argument will have to change based
on feature gate state.
@RomanBednar
enable Azure workload identity based on featuregate
ff5eb9b 
Operator needs to get a featuregate state and enable Azure workload
identity feature if the featuregate is set. We do this by adding a
placeholder string to --enable-azure-workload-identity injector flag
and replacing it's string value to "true" if the feature should be
enabled, and "false" otherwise.
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch 4 times, most recently from ce6f270 to f2670ca Compare June 21, 2023 09:25
@RomanBednar RomanBednar force-pushed the injector-workload-identity branch from f2670ca to ba6fd96 Compare June 21, 2023 09:26
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 21, 2023

@RomanBednar: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test e2e-azure
  • /test e2e-azure-csi
  • /test e2e-azure-csi-extended
  • /test images
  • /test unit
  • /test verify-deps

The following commands are available to trigger optional jobs:

  • /test e2e-azurestack-csi

Use /test all to run all jobs.

Details

In response to this:

/retest e2e-azure-csi-extended

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.

@RomanBednar
Copy link
Contributor Author

RomanBednar commented Jun 21, 2023

/test e2e-azure-csi-extended

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 21, 2023

@RomanBednar: 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
ci/prow/e2e-azurestack-csi ba6fd96 link false /test e2e-azurestack-csi

Full PR test history. Your PR dashboard.

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

@jsafrane
Copy link
Contributor

jsafrane commented Jun 22, 2023

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, RomanBednar

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2023
@openshift-merge-robot openshift-merge-robot merged commit af91b33 into openshift:master Jun 22, 2023
@stbenjam stbenjam mentioned this pull request Jun 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsafrane jsafrane jsafrane left review comments

@bertinatto bertinatto Awaiting requested review from bertinatto

@tsmetana tsmetana Awaiting requested review from tsmetana

Assignees

@jsafrane jsafrane

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@RomanBednar @openshift-ci-robot @jsafrane @openshift-merge-robot @jstuever