Skip to content

Conversation

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 27, 2025
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 27, 2025

<5> Specify the path to the output directory.
<6> Specify the path to the `serviceaccount-signer.public` file that you extracted from the cluster.
<7> Optional: By default, the `ccoctl` utility stores the OpenID Connect (OIDC) configuration files in a public S3 bucket and uses the S3 URL as the public OIDC endpoint. To store the OIDC configuration in a private S3 bucket that is accessed by the IAM identity provider through a public CloudFront distribution URL instead, use the `--create-private-s3-bucket` parameter.
====
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [error] AsciiDocDITA.TaskExample: Examples are allowed only once in DITA tasks.

<5> Specify the path to the output directory.
<6> Specify the path to the `serviceaccount-signer.public` file that you extracted from the cluster.
<7> Optional: By default, the `ccoctl` utility stores the OpenID Connect (OIDC) configuration files in a public S3 bucket and uses the S3 URL as the public OIDC endpoint. To store the OIDC configuration in a private S3 bucket that is accessed by the IAM identity provider through a public CloudFront distribution URL instead, use the `--create-private-s3-bucket` parameter.
====
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [error] AsciiDocDITA.ExampleBlock: Examples can not be inside of other blocks in DITA.

@sslocket sslocket changed the title OCPBUGS-61221# Ensure ccoctl uses the cluster's actual bound service account signing key OCPBUGS-61221-63545# Ensure new keypairs are not generated and public-key-file flag is used where appropriate Oct 27, 2025
@jstuever
Copy link

lgtm

@sslocket
Copy link
Contributor Author

@jianping-shu PTAL when you can.

@jianping-shu
Copy link

Will test the updated procedure after the PR of OCPBUGS-63541 is merged.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2025
@jianping-shu
Copy link

@sslocket @jstuever I had some review and testing.
My comments:
(1)For Rotating OIDC keys, either putting the serviceaccount-signer.public into the output_dir or specifying the public-key-file, the ccoctl command will work. I think it is good to make the changes to make the ccoctl cmds consistent in the documents.
(2)One error is "ccoctl azure create-managed-identities" doesn't have the parameter of "--public-key-file", I think the "ccoctl azure create-all" shall replace it, right?

--dnszone-resource-group-name <azure_dns_zone_resourcegroup_name> \// <5>
--installation-resource-group-name "${AZURE_INSTALL_RG}" <6>
--output-dir=<path_to_ccoctl_output_dir> \// <2>
--public-key-file=<path_to_ccoctl_output_dir>/serviceaccount-signer.public \// <3>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's the place for comment (2)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct, the ccoctl azure create-managed-identities does not have a --public-key-file parameter because it is not doing anything with the public keys. I didn't catch that it was a different command. We can remove the documentation of this parameter in this section.

@sslocket
Copy link
Contributor Author

sslocket commented Nov 4, 2025

@sslocket @jstuever I had some review and testing. My comments: (1)For Rotating OIDC keys, either putting the serviceaccount-signer.public into the output_dir or specifying the public-key-file, the ccoctl command will work. I think it is good to make the changes to make the ccoctl cmds consistent in the documents. (2)One error is "ccoctl azure create-managed-identities" doesn't have the parameter of "--public-key-file", I think the "ccoctl azure create-all" shall replace it, right?

@jstuever Is there any robustness advantage to including the --public-key-file flag in addition to --output-dir in these commands, or is it just completely redundant as I believe @jianping-shu is suggesting?

@jianping-shu
Copy link

It might be a good ides to keep the original "ccoctl azure create-managed-identities".
Because "ccoctl azure create-all" supports many parameters for resource names, it shall use the every parameters exactly same to those of the first run, which will need more edit changes.

@jstuever
Copy link

jstuever commented Nov 4, 2025

With the changes I made to ccoctl, the --public-key-file parameter will skip the code that generates a new key. The key generation code generates a new key when the private key does not exist. However, for security best practices, we don't want to force the users to download the private key from their cluster. The --public-key-file enables us to skip the key generation, removing the dependency on the private key altogether. As a result, the --public-key-file is required when only supplying the public key in order to avoid generating a new keypair. This can be demonstrated by deleting the private key in between invocations with and without the --public-key-file parameter specified.

openshift/cloud-credential-operator#933

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2025
@sslocket
Copy link
Contributor Author

sslocket commented Nov 5, 2025

@jianping-shu I've pushed the suggested revisions.

@jianping-shu
Copy link

LGTM

@sslocket
Copy link
Contributor Author

sslocket commented Nov 5, 2025

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Nov 5, 2025
@jeana-redhat jeana-redhat added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Nov 5, 2025
Copy link
Contributor

@jeana-redhat jeana-redhat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit but not required to fix for merge, excellent work!

/remove-label merge-review-in-progress
/remove-label merge-review-needed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new command steps should end with "by running the following command:"

@openshift-ci openshift-ci bot removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Nov 5, 2025
@openshift-ci
Copy link

openshift-ci bot commented Nov 5, 2025

@sslocket: 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-sigs/prow repository. I understand the commands that are listed here.

@jeana-redhat jeana-redhat merged commit e0a9066 into openshift:main Nov 5, 2025
2 checks passed
@jeana-redhat
Copy link
Contributor

/cherrypick enterprise-4.21
/cherrypick enterprise-4.20
/cherrypick enterprise-4.19
/cherrypick enterprise-4.18
/cherrypick enterprise-4.17
/cherrypick enterprise-4.16

@openshift-cherrypick-robot

@jeana-redhat: #101095 failed to apply on top of branch "enterprise-4.16":

Applying: OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate
Using index info to reconstruct a base tree...
A	modules/rotating-bound-service-keys.adoc
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): modules/rotating-bound-service-keys.adoc deleted in HEAD and modified in OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate. Version OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate of modules/rotating-bound-service-keys.adoc left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate

In response to this:

/cherrypick enterprise-4.21
/cherrypick enterprise-4.20
/cherrypick enterprise-4.19
/cherrypick enterprise-4.18
/cherrypick enterprise-4.17
/cherrypick enterprise-4.16

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.

@jeana-redhat
Copy link
Contributor

/cherrypick enterprise-4.21
/cherrypick enterprise-4.20

@jeana-redhat
Copy link
Contributor

/cherrypick enterprise-4.19
/cherrypick enterprise-4.18

@jeana-redhat
Copy link
Contributor

/cherrypick enterprise-4.17

@openshift-cherrypick-robot

@jeana-redhat: new pull request created: #101840

In response to this:

/cherrypick enterprise-4.21
/cherrypick enterprise-4.20

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-cherrypick-robot

@jeana-redhat: #101095 failed to apply on top of branch "enterprise-4.17":

Applying: OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate
Using index info to reconstruct a base tree...
A	modules/rotating-bound-service-keys.adoc
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): modules/rotating-bound-service-keys.adoc deleted in HEAD and modified in OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate. Version OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate of modules/rotating-bound-service-keys.adoc left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 OCPBUGS-61221-63545: Ensure new keypairs are not generated and public-key-file flag is used where appropriate

In response to this:

/cherrypick enterprise-4.17

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-cherrypick-robot

@jeana-redhat: new pull request created: #101841

In response to this:

/cherrypick enterprise-4.19
/cherrypick enterprise-4.18

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-cherrypick-robot

@jeana-redhat: new pull request created: #101842

In response to this:

/cherrypick enterprise-4.21
/cherrypick enterprise-4.20

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-cherrypick-robot

@jeana-redhat: new pull request created: #101843

In response to this:

/cherrypick enterprise-4.19
/cherrypick enterprise-4.18

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants