OSD-26415: Allow pull-secret to be updated w/o transferring ownership. #705
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
jira/valid-reference
|
@nephomaniac: This pull request references OSD-26415 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 the "4.19.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
@nephomaniac would you mind posting the output of the command running to the PR, just as an added validation? |
Contributor
Author
|
Example 'pull-secret rotation w/o ownership transfer ... |
Contributor
Author
|
Example output from ownership transfer. |
Contributor
Author
|
/label tide/merge-method-squash |
openshift-ci
bot
added
the
tide/merge-method-squash
tnierman
reviewed
Apr 24, 2025
tnierman
reviewed
Apr 24, 2025
tnierman
reviewed
Apr 24, 2025
cmd/cluster/transferowner.go
Outdated
|
|
||
| fmt.Println("Actual Cluster Pull Secret:") | ||
| blue.Println("Actual Cluster Pull Secret:") | ||
| fmt.Println(string(pullSecretData)) |
Member
There was a problem hiding this comment.
I know this wasn't introduced in this PR, but I'm a bit surprised that we just print the decoded pull-secret to stdout like this
Member
There was a problem hiding this comment.
@nephomaniac - you may have the most comprehension around this subcommand at the moment, do you know if this is necessary for the user to complete an owner transfer/pull-secret update task?
Member
There was a problem hiding this comment.
Mostly wondering if this is worth hiding behind a -v flag or something in a follow-up PR. That way we can still use the output if/when its needed, but it won't appear by default on screenshares or copy/pasted output
Contributor
Author
There was a problem hiding this comment.
This caught my attention too. I'm not sure why this was originally printed to the terminal, but I had assumed it was to allow the user one last chance to validate the marshaled data(?).
I can put this behind a -v or maybe prompt the user to display/hide.
I've added some programatic comparisons and the tool now displays the output of those in addition to the full secret dump. I think the results of this comparison can replace the printing to the screen for most cases.
Contributor
Author
There was a problem hiding this comment.
I've left the option to dump the secret data to the terminal, prompting the user to optionally do so with a warning. The newly added checks per auth section might provide enough info to the user executing this to make an informed decision about the status w/o the raw secret printed to the terminal.
openshift-merge-robot
added
the
needs-rebase
Contributor
|
Hello @nephomaniac Any update on this PR? |
openshift-merge-robot
removed
the
needs-rebase
|
@nephomaniac: This pull request references OSD-26415 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 the "4.20.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
Contributor
Author
|
/test verify-docs |
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
@nephomaniac: This pull request references OSD-26415 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 the "4.20.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
The example command brings wrong values for the "--gather" flag. Fixed with the available options.
Bumps [github.com/openshift/backplane-cli](https://github.com/openshift/backplane-cli) from 0.1.46 to 0.1.47. - [Release notes](https://github.com/openshift/backplane-cli/releases) - [Changelog](https://github.com/openshift/backplane-cli/blob/main/docs/release.md) - [Commits](openshift/backplane-cli@v0.1.46...v0.1.47) --- updated-dependencies: - dependency-name: github.com/openshift/backplane-cli dependency-version: 0.1.47 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Update docs Fix lint Update docs
…ands Updated all commands under cmd/ to consistently support the -C shorthand for the --cluster-id flag: - Added -C shorthand to commands using StringVar (no shorthand previously) - Standardized existing commands using -c (lowercase) to -C (uppercase) - Updated documentation to reflect the changes This ensures a consistent user experience across all osdctl commands that accept cluster ID parameters. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The LM (Last Month) case in getTimePeriod was using t.Month()-1 which doesn't handle year boundaries correctly. Changed to use AddDate(0, -1, 0) to properly calculate the previous month, fixing the failing test TestGetTimePeriod/Last_Month_(LM). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…penshift#784) * Bump osd-network-verifier and dependencies * Add pod mode support to osdctl network verify-egress Implement --pod-mode flag to run egress verification using Kubernetes Jobs instead of cloud instances, providing more accurate results by testing from within the actual cluster environment. Key features: - New --pod-mode flag enables Kubernetes-based verification - Automatic region detection from OCM for AWS clusters - Manual region override with --region flag - Configurable namespace with --namespace flag (default: openshift-network-diagnostics) - Custom kubeconfig support with --kubeconfig flag - Automatic probe switching to curl (required for pod mode) - Comprehensive input validation with clear error messages - Mutual exclusivity with cloud-specific flags Benefits: - No cloud credentials required - Tests actual cluster network environment - More accurate results than external instance testing - Secure execution with restrictive pod security contexts Usage examples: osdctl network verify-egress --cluster-id my-cluster --pod-mode osdctl network verify-egress --pod-mode --platform aws-classic --region us-east-1 Includes comprehensive unit test coverage with 21 new tests covering: - Input validation logic - Region detection (OCM vs manual) - Probe validation and switching - AWS config generation - Error handling scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Use backplane as preference for getting k8s credentials * Docs and fmt * Consolidate and cleanup tests * Pull in actual osd-network-verifier release, update region flag --------- Co-authored-by: Claude <[email protected]>
…t#787) * add basic link verification and tests * fix skip-link-check flag * move linkValidator to pkg * add warning for non fatal HTTP response codes * update documentation with --skip-link-check flag * update pkg name to snake case * add newTestServer helper for link_validator tests
…t#788) * https://issues.redhat.com/browse/SREP-861 Add support running network-verifier in pod with a service account. Add a flag to disable sending a service log. * Refine Cursor generated code * Fix if/else syntax * Update docs with make generate-docs * Add unit tests for using a kubeconfig and service account * change write to 0600 * Update PR based on feedback: Extract REST config logic from setupForPodMode and update priority to respect explicit user choices (--kubeconfig > --cluster-id > ServiceAccount > default). Add serviceAccountTokenPath constant and comprehensive tests. Change --no-service-log flag to --skip-service-log * fix nosec issue and update docs * change SL flag to skipServiceLog --------- Co-authored-by: Dakota Long <[email protected]>
…#760) * SREP-217: Changed cluster-id to id-cluster * ADD: Username Filter for OSDCTL Write Events Still Missing ErrorChecking and Showing errors if no matching username * UPDATE: Added Print Username Not recognized if nothing found * ADD: Events Filter * ADD: Resource Name and Type to PrintEvents * ADD: Resource Type Filters * Update: Resource Type and Name Both Working * REFACTOR: Refactored AWS.go Write-events.go Refactored aws.go to use Case-Switching to prevent duplications of Code Used Slices for Filters * REFACTOR: Created Filter Function Moved Filtering into its own separate function * ADD: Exclusion Filters More said in documentation * ADD: Filter by list and Print out Errors * ADD: Filters with ARN * Test * ADD: Day and Weeks to --since flag FIX: permission_denied.go now uses GetEvents() CLEANUP: Comment and Code Cleanup ADD: Filters for write-event.go REFACTOR: Filters.go CLEANUP: Code Cleanup, Variable Renaming ADD: Sample code in flags REFACTOR & FIX: Code cleanup in cloudtrail & filter.go refactored NEW CODE: Remodel filter function to use lambda for readability REFACTOR: Refactored Files To Provide Linear Imports FIX: Issues with go.mod FEAT: Added ValidateFilters before any time requiring operation are required REFACTOR: Added filter package REMOVE: util.go FEAT: Added functionality to inlcude resource-name & type This is mainly because permission denied also requires PrintEvent FIX: testdata go files had issues with imports REFACTOR: removed event, aws and filter packages to reduce complexity. FEAT: Added EndTime for --until flag FEAT: Added --print-format to allow users to specify which fields to print REFACTOR: Added time.go for --until --after --since flag DONE: Final Cleanup for SREP-217 & Documentation --------- Co-authored-by: openshift-merge-bot[bot] <148852131+openshift-merge-bot[bot]@users.noreply.github.com>
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nephomaniac The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/retest |
|
@nephomaniac: This pull request references OSD-26415 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 the "4.20.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
Contributor
Author
|
/retest |
Contributor
|
@nephomaniac: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Contributor
Author
|
Closing this older PR per PR794 . Newer PR794 does not depend on backplane cli changes to support multiple OCM contexts for hive, allowing non-prod testing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR attempts to allow a user to update a cluster's pull secret w/o transferring ownership for both classic and HCP clusters.
Example usage: