CNTRLPLANE-1710: feat(globalps): security enhancements on GlobalPullSecret feature - #7234
Conversation
- Enable global pull secret for ROSA HCP. - Enabled E2E for AWS plaform Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Replace 'privileged: true' with least privilege security context in the Global Pull Secret DaemonSet. The container now uses specific capabilities (DAC_OVERRIDE, SYS_ADMIN) instead of full host privileges while maintaining the ability to modify kubelet config and restart the kubelet service. Security improvements: - Remove privileged: true - Add only required capabilities: DAC_OVERRIDE, SYS_ADMIN - Drop all other capabilities - Enable read-only root filesystem - Disable privilege escalation Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
…ze sync-global-pullsecret - Create original-pull-secret in DataPlane namespace for direct file access - Mount both original and global pull secrets as volumes in DaemonSet pods - Add configuration hash to DaemonSet labels to trigger pod recreation on content changes - Update sync-global-pullsecret to read secrets from mounted files instead of use the API This change improves performance by eliminating Kubernetes API calls for secret reading and ensures pods are automatically recreated when pull secret content changes. Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
…approach - Replace controller manager with simple loop-based sync in sync-global-pullsecret - Mount original-pull-secret and global-pull-secret as files in DaemonSet - Use configuration hash in DaemonSet labels to trigger pod recreation on secret changes - Simplify RBAC by using default ServiceAccount instead of custom ServiceAccount/Role/RoleBinding - Remove Kubernetes API dependencies from sync-global-pullsecret for better performance - Update tests to work with new GlobalPullSecretSyncer structure This optimization eliminates the need for Kubernetes informers and reduces resource usage while maintaining the same functionality for pull secret synchronization across cluster nodes. Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
…issions - Reverted DaemonSet to Privilleged mode, it does not work just with capabilities or any other limitations - Add precedence logic for GlobalPullSecret merge based on managed services detection - Fix DaemonSet selector immutability issues - Enable syncer access to host files for pull secret synchronization - Add comprehensive test coverage for precedence scenarios - Fix error handling for missing global pull secret files The GlobalPullSecret now respects different precedence rules: - For managed services: original pull secret entries take precedence - For non-managed services: user-provided pull secret entries take precedence Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
- Enable global pull secret for ROSA HCP. - Enabled E2E for AWS plaform Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
…GlobalPullSecret - Add logic to preserve trailing newlines when updating kubelet config.json - Refactor EnsureGlobalPullSecret E2E test to run as proper subtest - Add validation for NodePool upgrade type compatibility This fix ensures kubelet config files maintain their original formatting when updated by the global pull secret syncer, preventing potential configuration inconsistencies. Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
…est scale Improves autoscaling test robustness and scale to better validate behavior: Configuration changes: - Use RandomExpander instead of LeastWasteExpander for better distribution probability - Increase MaxFreeDifferenceRatioPercent to 70% for more permissive balancing - Set m5.xlarge instance types to ensure adequate memory capacity - Increase MaxNodesTotal from 4 to 6 nodes for larger scale testing Test validation improvements: - Increase workload from 4 to 6 jobs to match node scaling - Relax balancing check to accept 2+4, 3+3, 4+2 distributions (≥2 nodes per NodePool) - Reject extreme imbalances (≤1 nodes in any NodePool) - Update comments and log messages to reflect new expectations The cluster-autoscaler behavior is correct - it doesn't guarantee perfect balance, only reasonable distribution within the configured threshold. Test expectations now align with actual autoscaler behavior based on cluster state analysis. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
openshift-ci-robot
added
jira/valid-reference
|
@jparrill: This pull request references Jira Issue OCPBUGS-59649, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. This pull request references CNTRLPLANE-1398 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.20." or "openshift-4.20.", but it targets "openshift-4.21" instead. 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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
area/control-plane-operator
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
area/documentation
|
/jira cherrypick OCPBUGS-59649 |
|
@jparrill: Jira Issue OCPBUGS-59649 has been cloned as Jira Issue OCPBUGS-65615. Will retitle bug to link to clone. 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. |
openshift-ci
Bot
changed the title
|
@jparrill: This pull request references Jira Issue OCPBUGS-65615, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
jparrill
changed the title
|
@jparrill: This pull request references Jira Issue OCPBUGS-65615, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. This pull request references CNTRLPLANE-1398 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.20." or "openshift-4.20.", but it targets "openshift-4.21" instead. 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. |
|
/jira refresh |
|
@jparrill: This pull request references Jira Issue OCPBUGS-65615, which is invalid:
Comment This pull request references CNTRLPLANE-1398 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.20." or "openshift-4.20.", but it targets "openshift-4.21" instead. 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. |
|
@jparrill I did the pre-merge test with pr: openshift/release#71678, the step of checking global pull secret passed and the e2e-aws-ovn-conformance tests passed too. |
|
/retest-required |
|
@jparrill: This pull request references CNTRLPLANE-1710 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.z" version, but no target version was set. Retaining the jira/valid-bug label as it was manually added. 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. |
|
/retest-required |
|
@jparrill: This pull request references CNTRLPLANE-1710 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.z" version, but no target version was set. Retaining the jira/valid-bug label as it was manually added. 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. |
|
@jparrill: This pull request references CNTRLPLANE-1710 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.z" version, but no target version was set. Retaining the jira/valid-bug label as it was manually added. 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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Bug: Nodes not unlabeled when transitioning to InPlace
The labelNodesForGlobalPullSecret function only labels nodes that should receive the GlobalPullSecret DaemonSet, but it doesn't remove the label from nodes that previously had it when they transition from Replace to InPlace upgrade strategy. This causes nodes to retain the nodepool-globalps-enabled label even after becoming InPlace nodes, allowing the DaemonSet to incorrectly schedule on them. The function should remove the label from nodes that are NOT in the nodesToLabel set.
|
@jparrill: This pull request references CNTRLPLANE-1710 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.z" version, but no target version was set. Retaining the jira/valid-bug label as it was manually added. 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. |
|
/retest-required |
3 similar comments
|
/retest-required |
|
/retest-required |
|
/retest-required |
|
@jparrill is |
|
/retest-required |
Should not tbh. Discussing with Cesar and Tony to override them. |
|
/override e2e-aks-4-19 |
|
/override e2e-aws-4-19 |
|
@jparrill: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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 kubernetes-sigs/prow repository. |
|
@jparrill: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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 kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-aks-4-19 |
|
/override ci/prow/e2e-aws-4-19 |
|
Override discussion on https://redhat-internal.slack.com/archives/G01QS0P2F6W/p1764079137876449 |
|
@jparrill: Overrode contexts on behalf of jparrill: ci/prow/e2e-aks-4-19 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 kubernetes-sigs/prow repository. |
|
@jparrill: Overrode contexts on behalf of jparrill: ci/prow/e2e-aws-4-19 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 kubernetes-sigs/prow repository. |
|
@jparrill: all tests passed! 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. |
a5aab25
into
openshift:release-4.20
6 participants
What this PR does / why we need it:
Manual backport of:
Which issue(s) this PR fixes:
Depending on
Note
Hardens the Global Pull Secret feature by targeting only eligible nodes, preferring original credentials, rewriting the node sync agent to use mounted secrets with atomic updates, enabling the controller on AWS, and updating docs/tests accordingly.
globalpsonAWSin addition toAzure.nodeSelector(hypershift.openshift.io/nodepool-globalps-enabled=true).kube-system/original-pull-secret; create/updateglobal-pull-secretonly when additional secret exists; add pod config hash label for rollouts.openshift-user-criticalpriority, reduce resources, mount secrets and kubelet/dbus via explicit volumes.sync-global-pullsecret):original/globalsecrets, validates JSON, preserves trailing newline, writes atomically, and restarts kubelet via dbus with retries and rollback.OriginalPullSecretmanifest; remove RBAC-related helpers.util.CountAvailableNodeswith tests.Written by Cursor Bugbot for commit db0f6e4. This will update automatically on new commits. Configure here.