Implement ccoctl command to create infrastructure required for Azure workload identity #523

abutcher · 2023-03-21T21:03:18Z

Implement ccoctl azure sub-commands which will create an RSA key pair, OIDC Azure blob container infrastructure and user-assigned managed identities for processed CredentialsRequests which must contain .spec.serviceAccountNames to receive federated identity credentials for the listed service accounts.

➜  ccoctl azure -h
Creating/updating/deleting cloud credentials objects for Azure

Usage:
  ccoctl azure [command]

Available Commands:
  create-all                Create OIDC issuer and managed identities
  create-key-pair           Create a key pair
  create-managed-identities Create Azure Managed Identities
  create-oidc-issuer        Create OIDC Issuer
  delete                    Delete OIDC issuer and managed identities

Flags:
  -h, --help   help for azure

Use "ccoctl azure [command] --help" for more information about a command.

ccoctl azure create-key-pair

➜  ccoctl azure create-key-pair --output-dir /tmp/ccoctl-output

ccoctl azure create-oidc-issuer

OIDC issuer infrastructure will be created in a resource group with a name derived from --name when no --oidc-resource-group-name is provided. By default this OIDC resource group will be named <--name parameter> + "-oidc", eg "abutchertest-oidc" but may be explicitly named by providing an --oidc-resource-group-name parameter.

➜  ccoctl azure create-oidc-issuer \
        --name abutchertest \
        --output-dir /tmp/ccoctl-output \
        --region centralus \
        --subscription-id <subscription-id> \
        --public-key-file /tmp/ccoctl-output/serviceaccount-signer.public
        [--oidc-resource-group-name <oidc rg name>]
        [--storage-account-name <storage account name>]
        [--user-tags key1=value1,key2=value2]

Note: The storage account name has more strict requirements than that of a resource group name. For example, if the storage account name derived from the --name parameter is invalid the command will exit earlier with an error. If specific naming is required --storage-account-name can be specified explicitly.

➜  ccoctl azure create-oidc-issuer --name abutcher-demo \
          --output-dir /tmp/ccoctl-output \
          --region centralus \
          --subscription-id <subscription-id> \
          --public-key-file /tmp/ccoctl-output/serviceaccount-signer.public \
          --user-tags cheese=beans,corn=rice
2023/05/31 10:40:37 No --oidc-resource-group-name provided, defaulting OIDC resource group name to abutcher-demo-oidc
2023/05/31 10:40:37 No --storage-account-name provided, defaulting storage account name to abutcher-demo
2023/05/31 10:40:37 invalid storage account name: abutcher-demo. Azure storage account names must be between 3 and 24 characters in length and may contain numbers and lowercase letters only.

ccoctl azure create-managed-identities

ccoctl will grant permissions to created user-assigned managed identities within the scope of an "installation" resource group and this resource group must be used as the resource group configured for future cluster installation. By default this installation resource group will be named <--name parameter>, eg "abutchertest" but may be explicitly named by providing an --installation-resource-group-name parameter.

Note: The OpenShift installer requires that the installation resource group be entirely empty so ccoctl just creates the resource group such that the resource group can be used for scoping user-assigned managed identities and instructs that this resource group MUST be used for cluster installation. Being able to provide the installation resource group for scoping allows us to use ccoctl to create OIDC/managed identity infrastructure for an existing cluster to assist with testing.

In order to scope cluster ingress operations, ccoctl must also be provided the --dnszone-resource-group-name which is the name of the resource group in which the future cluster's base domain DNS zone exists (as provided to the OpenShift installer via the install-config.yaml).

➜   ccoctl azure create-managed-identities \
         --name abutchertest \
         --output-dir /tmp/ccoctl-output \
         --region centralus \
         --subscription-id <subscription-id> \
         --credentials-requests-dir /tmp/azure-crs \
         --resource-group abutchertest-infra \
         --issuer-url https://abutchertest.blob.core.windows.net/abutchertest/ \
         --dnszone-resource-group-name <future cluster's base domain resource group name>
         [--oidc-resource-group-name <oidc rg name>]
         [--installation-resource-group-name <installation rg name>]
         [--user-tags key1=value1,key2=value2]

ccoctl azure create-all

ccoctl azure create-all combines create-key-pair, create-oidc-issuer and create-managed-identities into a single command.

➜   ccoctl azure create-all \
         --name abutchertest \
         --output-dir /tmp/ccoctl-output \
         --region centralus \
         --subscription-id <subscription-id> \
         --credentials-requests-dir /tmp/azure-crs \
         --dnszone-resource-group-name <future cluster's base domain resource group name> \
         [--oidc-resource-group-name <oidc rg name>]
         [--installation-resource-group-name <installation rg name>]
         [--storage-account-name <storage account name>]
         [--user-tags key1=value1,key2=value2]

ccoctl azure delete

The delete subcommand deletes the storage account, blob container and managed identities from the OIDC resource group but will not delete the OIDC resource group unless requested. Note that as above, the OIDC resource group name may be specified by --oidc-resource-group-name but is defaulted to <--name parameter> + "-oidc".

➜   ccoctl azure delete \
         --name abutchertest \
         --region centralus \
         --subscription-id <subscription-id>
         [--oidc-resource-group-name <oidc rg name>]
         [---delete-oidc-resource-group]

CCO-232
openshift/enhancements#1301

codecov · 2023-03-21T23:15:06Z

Codecov Report

Merging #523 (7ba2acc) into master (38e7f96) will decrease coverage by 0.27%.
The diff coverage is 43.09%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
- Coverage   48.80%   48.54%   -0.27%     
==========================================
  Files          88       93       +5     
  Lines        9853    11422    +1569     
==========================================
+ Hits         4809     5545     +736     
- Misses       4469     5259     +790     
- Partials      575      618      +43

Impacted Files	Coverage Δ
pkg/cmd/provisioning/azure/azure.go	`0.00% <0.00%> (ø)`
pkg/cmd/provisioning/azure/create_all.go	`0.00% <0.00%> (ø)`
pkg/cmd/provisioning/azure/delete.go	`0.00% <0.00%> (ø)`
pkg/azure/clients.go	`4.93% <5.44%> (+4.93%)`	⬆️
pkg/cmd/provisioning/azure/create_oidc_issuer.go	`53.21% <53.21%> (ø)`
pkg/cmd/provisioning/utils.go	`77.77% <57.14%> (+0.32%)`	⬆️
...md/provisioning/azure/create_managed_identities.go	`57.62% <57.62%> (ø)`
pkg/azure/mock/client_generated.go	`71.20% <69.00%> (-28.80%)`	⬇️

... and 1 file with indirect coverage changes

abutcher · 2023-03-30T19:55:20Z

UT, create-all and delete sub-commands still on the way but would appreciate comments on what I have so far.

/assign @jstuever
/cc @2uasimojo

jstuever

Some thoughts during a partial review...

pkg/cmd/provisioning/azure/create_identity_provider.go

RomanBednar · 2023-04-21T09:45:07Z

@abutcher Does this statement still stand? Can we expect to get azure_federated_token_file in the cloud credentials secret or do we have to assume the token path /var/run/secrets/openshift/serviceaccount?

abutcher · 2023-05-08T18:57:13Z

@RomanBednar We'll set azure_federated_token_file in the secret as /var/run/secrets/openshift/serviceaccount/token.

cloud-credential-operator/pkg/cmd/provisioning/azure/create_managed_identities.go

Line 253 in 565bf56

    
           fileData := fmt.Sprintf(secretManifestTemplate, clientID, tenantID, region, subscriptionID, provisioning.OidcTokenPath, cr.Spec.SecretRef.Name, cr.Spec.SecretRef.Namespace)

cloud-credential-operator/pkg/cmd/provisioning/azure/create_managed_identities.go

Line 42 in 565bf56

azure_federated_token_file: %s

cloud-credential-operator/pkg/cmd/provisioning/constants.go

Line 17 in 565bf56

OidcTokenPath = "/var/run/secrets/openshift/serviceaccount/token"

abutcher · 2023-05-22T19:10:51Z

/test e2e-upgrade

pkg/cmd/provisioning/azure/delete.go

docs/azure-workload-identity.md

jstuever · 2023-05-23T18:36:24Z

/test verify

abutcher · 2023-05-23T21:02:23Z

/test verify

…structure.

… to create Azure infrastructure.

… workload identity.

…t data from implementation.

…ready using it.

… usage.

jeana-redhat · 2023-06-15T18:31:54Z

/label docs-approved
Great explanations here and in the enhancement, tysm!

jstuever · 2023-06-20T20:58:31Z

/test e2e-azure-manual-oidc

jstuever · 2023-06-21T21:15:42Z

/test e2e-azure-manual-oidc

jstuever · 2023-06-23T21:50:43Z

/test e2e-azure-manual-oidc

jstuever · 2023-06-23T22:34:42Z

/test e2e-azure-manual-oidc

jstuever · 2023-06-23T22:58:39Z

/test e2e-azure-manual-oidc

jstuever · 2023-06-23T23:06:54Z

/test e2e-azure-manual-oidc

openshift-ci · 2023-06-24T00:54:28Z

@abutcher: 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-azure-manual-oidc	`7ba2acc`	link	false	`/test e2e-azure-manual-oidc`

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.

jstuever · 2023-06-26T16:27:05Z

e2e-azure-manual-oidc did what we expected at this point. It was able to create the oidc resources with ccoctl azure create-all, and begin installing a cluster. It failed because not all of the operators at this point in time have the necessary changes to support oidc. As a result, those operators failed because their client authentication secrets do not contain a clientSecret.

jstuever

/lgtm

openshift-ci · 2023-06-26T23:24:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abutcher, jstuever

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~~ [abutcher,jstuever]

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

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 Mar 21, 2023

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

openshift-ci bot requested review from dlom and jstuever March 21, 2023 21:05

abutcher force-pushed the CCO-232 branch from 423c121 to 779d4d6 Compare March 21, 2023 21:05

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 21, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 21, 2023

abutcher force-pushed the CCO-232 branch from 779d4d6 to 4c6b66b Compare March 28, 2023 15:30

openshift-ci bot assigned jstuever Mar 30, 2023

openshift-ci bot requested a review from 2uasimojo March 30, 2023 19:55

abutcher force-pushed the CCO-232 branch from ba8f8e1 to c1f8810 Compare April 3, 2023 20:03

jstuever reviewed Apr 5, 2023

View reviewed changes

pkg/cmd/provisioning/azure/create_identity_provider.go Outdated Show resolved Hide resolved

pkg/cmd/provisioning/azure/create_identity_provider.go Outdated Show resolved Hide resolved

pkg/cmd/provisioning/azure/create_identity_provider.go Outdated Show resolved Hide resolved

abutcher commented Apr 5, 2023

View reviewed changes

pkg/cmd/provisioning/azure/create_identity_provider.go Outdated Show resolved Hide resolved

abutcher force-pushed the CCO-232 branch from e03fe64 to 71e4a55 Compare April 7, 2023 20:59

RomanBednar mentioned this pull request Apr 26, 2023

CCO-324,CCO-325: add support for workload identity openshift/cluster-cloud-controller-manager-operator#245

Merged

abutcher mentioned this pull request May 9, 2023

NE-1244: Use permissions instead of the "Contributor" role in Azure CredentialsRequest openshift/cluster-ingress-operator#929

Merged

jstuever reviewed May 23, 2023

View reviewed changes

pkg/cmd/provisioning/azure/delete.go Outdated Show resolved Hide resolved

jstuever reviewed May 23, 2023

View reviewed changes

docs/azure-workload-identity.md Outdated Show resolved Hide resolved

abutcher added 5 commits May 31, 2023 10:29

Include azure-sdk-for-go dependencies necessary to create Azure infra…

66e92a2

…structure.

Vendor updates from including azure-sdk-for-go dependencies necessary…

9a7a09a

… to create Azure infrastructure.

Implement ccoctl commands to create infrastructure required for Azure…

f16de31

… workload identity.

Add AzureClientWrapper.MockStorageClientBeginCreateResp to remove tes…

4443fe9

…t data from implementation.

s/PollerWrapper.generic/PollerWrapper.mockResp/g

0636cab

openshift-ci bot requested a review from suhanime June 14, 2023 13:08

Remove unused mockRoleDefinitionGetByIDSuccess func.

695b862

abutcher changed the title ~~WIP: Implement ccoctl command to create infrastructure required for Azure workload identity~~ Implement ccoctl command to create infrastructure required for Azure workload identity Jun 14, 2023

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 14, 2023

abutcher added 4 commits June 14, 2023 10:59

Remove github.com/gofrs from go mod/sum.

5eb0f0f

Remove github.com/gofrs from vendor directory.

758c7b6

Replace github.com/gofrs with github.com/google/uuid since we were al…

62028d2

…ready using it.

Correct --credentials-requests-dir usage in 'ccoctl azure create-all'…

6dc2cef

… usage.

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

abutcher added 3 commits June 20, 2023 10:03

Bump armautorization to v2.

27b3a7b

Vendor changes from bumping armauthorization to v2.

7286cda

Convert armauthorization NewListPager to v2 NewListForScopePager.

0d96ce4

abutcher force-pushed the CCO-232 branch from 87db668 to 0d96ce4 Compare June 20, 2023 14:05

abutcher added 4 commits June 20, 2023 13:10

remove armauthorization from go mod/sum

76c7b29

remove armauthorization from vendor

5689bf0

Fix tests for armauthorization -> armauthorization/v2 swap.

0fb9e29

Remove log added for debugging role assignment removal.

7ba2acc

jstuever approved these changes Jun 26, 2023

View reviewed changes

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

openshift-merge-robot merged commit ee67cc6 into openshift:master Jun 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ccoctl command to create infrastructure required for Azure workload identity #523

Implement ccoctl command to create infrastructure required for Azure workload identity #523

abutcher commented Mar 21, 2023 •

edited

Loading

codecov bot commented Mar 21, 2023 •

edited

Loading

abutcher commented Mar 30, 2023

jstuever left a comment

RomanBednar commented Apr 21, 2023

abutcher commented May 8, 2023

abutcher commented May 22, 2023

jstuever commented May 23, 2023

abutcher commented May 23, 2023

jeana-redhat commented Jun 15, 2023

jstuever commented Jun 20, 2023

jstuever commented Jun 21, 2023

jstuever commented Jun 23, 2023

jstuever commented Jun 23, 2023

jstuever commented Jun 23, 2023

jstuever commented Jun 23, 2023

openshift-ci bot commented Jun 24, 2023

jstuever commented Jun 26, 2023 •

edited

Loading

jstuever left a comment

openshift-ci bot commented Jun 26, 2023

Implement ccoctl command to create infrastructure required for Azure workload identity #523

Implement ccoctl command to create infrastructure required for Azure workload identity #523

Conversation

abutcher commented Mar 21, 2023 • edited Loading

ccoctl azure create-key-pair

ccoctl azure create-oidc-issuer

ccoctl azure create-managed-identities

ccoctl azure create-all

ccoctl azure delete

codecov bot commented Mar 21, 2023 • edited Loading

Codecov Report

abutcher commented Mar 30, 2023

jstuever left a comment

Choose a reason for hiding this comment

RomanBednar commented Apr 21, 2023

abutcher commented May 8, 2023

abutcher commented May 22, 2023

jstuever commented May 23, 2023

abutcher commented May 23, 2023

jeana-redhat commented Jun 15, 2023

jstuever commented Jun 20, 2023

jstuever commented Jun 21, 2023

jstuever commented Jun 23, 2023

jstuever commented Jun 23, 2023

jstuever commented Jun 23, 2023

jstuever commented Jun 23, 2023

openshift-ci bot commented Jun 24, 2023

jstuever commented Jun 26, 2023 • edited Loading

jstuever left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jun 26, 2023

abutcher commented Mar 21, 2023 •

edited

Loading

codecov bot commented Mar 21, 2023 •

edited

Loading

jstuever commented Jun 26, 2023 •

edited

Loading