Add OCP 5.0 Gap Analysis files#2759
Open
rosa-gap-analysis-bot wants to merge 2 commits into
Open
Conversation
- AWS STS IAM policies: 30 file(s) - GCP WIF templates: 1 file(s) - Acknowledgment files: 4 file(s) Addresses gap-analysis validation failures for OCP 5.0. Co-Authored-By: ROSA Gap Analysis Bot <rosa-gap-analysis-bot@redhat.com}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis pull request adds complete infrastructure configuration for OpenShift 5.0 support using Secure Token Service (STS) for AWS and Workload Identity Federation (WIF) for GCP. It includes deployment targeting configurations, 27 AWS IAM credential policies for operators and system components, and a comprehensive GCP workload identity federation template with service account mappings. ChangesOpenShift 5.0 STS and WIF Identity Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rosa-gap-analysis-bot 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 |
openshift-ci
Bot
added
the
needs-ok-to-test
Contributor
|
Hi @rosa-gap-analysis-bot. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
Member
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
resources/sts/5.0/sts_installer_permission_policy.json (1)
69-70: 💤 Low valueDeprecated EC2-Classic actions can be removed.
ec2:DescribeVpcClassicLinkandec2:DescribeVpcClassicLinkDnsSupportreference EC2-Classic features that AWS retired in August 2022. These actions are no longer functional and can be safely removed to reduce policy clutter.Proposed cleanup
"ec2:DescribeVpcAttribute", - "ec2:DescribeVpcClassicLink", - "ec2:DescribeVpcClassicLinkDnsSupport", "ec2:DescribeVpcEndpoints",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/sts/5.0/sts_installer_permission_policy.json` around lines 69 - 70, Remove the two deprecated EC2-Classic actions "ec2:DescribeVpcClassicLink" and "ec2:DescribeVpcClassicLinkDnsSupport" from the JSON "Action" array in the policy (they are the entries currently listed as ec2:DescribeVpcClassicLink and ec2:DescribeVpcClassicLinkDnsSupport); delete those strings and adjust commas so the JSON array remains valid (no trailing commas), then validate the policy JSON structure and re-run any linter/formatter to ensure formatting is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/osd-cluster-acks/sts/5.0/config.yaml`:
- Around line 1-9: Add an explicit resourceApplyMode entry at the same level as
deploymentMode (e.g., resourceApplyMode: <desired-mode>) and update the
selectorSyncSet.matchExpressions to include an exclusion for fedramp by adding a
matchExpressions item with key: api.openshift.com/fedramp, operator: NotIn,
values: ["true"]; keep the existing hive.openshift.io/version-major-minor and
api.openshift.com/sts entries unchanged so the deploymentMode, selectorSyncSet,
and resourceApplyMode (top-level) together drive targeting/apply behavior.
In `@deploy/osd-cluster-acks/wif/5.0/config.yaml`:
- Around line 1-9: Add an explicit resourceApplyMode key under the top-level
(e.g., resourceApplyMode: Sync) and extend the selectorSyncSet.matchExpressions
to include an exclusion for the FedRAMP label by adding a matchExpression with
key: api.openshift.com/fedramp, operator: NotIn, values: ["true"]; update the
existing selectorSyncSet block (which currently contains matchExpressions with
keys hive.openshift.io/version-major-minor and api.openshift.com/wif) to include
this new fedramp exclusion so rollout semantics and compliance targeting are
complete.
In `@resources/sts/5.0/openshift_aws_vpce_operator_avo_aws_creds_policy.json`:
- Around line 29-53: The policy currently grants
route53:ChangeResourceRecordSets unconditionally and again in the statement with
Sid "Route53ManageRecords" (which includes domain restrictions), making the
conditional ineffective; remove "route53:ChangeResourceRecordSets" from the
unconditional Actions array (the earlier statement that also includes
"route53:ListHostedZonesByName" and "route53:ListResourceRecordSets") so that
only the "Route53ManageRecords" statement controls ChangeResourceRecordSets with
its Condition limiting domains.
In `@resources/sts/5.0/openshift_kube_controller_manager_credentials_policy.json`:
- Around line 28-44: The policy JSON contains a duplicated action string
"elasticloadbalancing:AddTags" in the actions array; remove the redundant entry
so each IAM action appears only once (keep a single
"elasticloadbalancing:AddTags" and delete the other) within the actions list in
openshift_kube_controller_manager_credentials_policy.json to avoid the
copy-paste duplicate.
In `@resources/sts/5.0/osd_scp_policy.json`:
- Around line 35-148: The policy currently uses overly broad wildcards (e.g.,
"iam:*", "kms:*", "sts:*", "cloudwatch:*", "events:*", "logs:*", "route53:*",
"tag:*", "servicequotas:*") with Resource:"*", which violates least-privilege;
update each statement (e.g., Sids: "AllowIAM", "AllowKMS", "AllowSTS",
"AllowCloudWatch", "AllowCloudWatchEvents", "AllowCloudWatchLogs",
"AllowRoute53", "AllowTag", "AllowServiceQuotas", "AllowELB", "AllowSupport") to
enumerate only the specific actions needed and narrow Resource ARNs (or use
condition keys) instead of "*" — for example replace "iam:*" with specific calls
(ListRoles, PassRole, CreateRole, etc.) scoped to required role ARNs, restrict
"kms:*" to the specific kms:Encrypt/Decrypt on the KMS key ARNs, limit "sts:*"
to sts:AssumeRole with specific role ARNs, and similarly scope
CloudWatch/Logs/Events/Route53 actions to the exact log groups, event rules, or
hosted zones required; apply this pattern across each Sid to enforce
least-privilege.
In `@resources/wif/5.0/vanilla.yaml`:
- Around line 563-564: The permission list for the role sre_managed_support
contains a duplicate entry "logging.logEntries.list"; remove the redundant
"logging.logEntries.list" occurrence from the permissions array in the
sre_managed_support role so it only appears once (search for the permissions
block under the sre_managed_support role and delete the duplicate string).
---
Nitpick comments:
In `@resources/sts/5.0/sts_installer_permission_policy.json`:
- Around line 69-70: Remove the two deprecated EC2-Classic actions
"ec2:DescribeVpcClassicLink" and "ec2:DescribeVpcClassicLinkDnsSupport" from the
JSON "Action" array in the policy (they are the entries currently listed as
ec2:DescribeVpcClassicLink and ec2:DescribeVpcClassicLinkDnsSupport); delete
those strings and adjust commas so the JSON array remains valid (no trailing
commas), then validate the policy JSON structure and re-run any linter/formatter
to ensure formatting is unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ac65caeb-f276-4c1d-af06-b369e48866a1
⛔ Files ignored due to path filters (3)
hack/00-osd-managed-cluster-config-integration.yaml.tmplis excluded by!hack/**hack/00-osd-managed-cluster-config-production.yaml.tmplis excluded by!hack/**hack/00-osd-managed-cluster-config-stage.yaml.tmplis excluded by!hack/**
📒 Files selected for processing (35)
deploy/osd-cluster-acks/sts/5.0/config.yamldeploy/osd-cluster-acks/sts/5.0/osd-sts-ack_CloudCredential.yamldeploy/osd-cluster-acks/wif/5.0/config.yamldeploy/osd-cluster-acks/wif/5.0/osd-wif-ack_CloudCredential.yamlresources/sts/5.0/openshift_aws_vpce_operator_avo_aws_creds_policy.jsonresources/sts/5.0/openshift_capa_controller_manager_credentials_policy.jsonresources/sts/5.0/openshift_cloud_credential_operator_cloud_credential_operator_iam_ro_creds_policy.jsonresources/sts/5.0/openshift_cloud_ingress_operator_cloud_credentials_policy.jsonresources/sts/5.0/openshift_cloud_network_config_controller_cloud_credentials_policy.jsonresources/sts/5.0/openshift_cluster_csi_drivers_ebs_cloud_credentials_policy.jsonresources/sts/5.0/openshift_control_plane_operator_credentials_policy.jsonresources/sts/5.0/openshift_image_registry_installer_cloud_credentials_policy.jsonresources/sts/5.0/openshift_ingress_operator_cloud_credentials_policy.jsonresources/sts/5.0/openshift_kms_provider_credentials_policy.jsonresources/sts/5.0/openshift_kube_controller_manager_credentials_policy.jsonresources/sts/5.0/openshift_machine_api_aws_cloud_credentials_policy.jsonresources/sts/5.0/operator_iam_role_policy.jsonresources/sts/5.0/osd_scp_policy.jsonresources/sts/5.0/shared_vpc_openshift_ingress_operator_cloud_credentials_policy.jsonresources/sts/5.0/sts_installer_core_permission_boundary_policy.jsonresources/sts/5.0/sts_installer_permission_policy.jsonresources/sts/5.0/sts_installer_privatelink_permission_boundary_policy.jsonresources/sts/5.0/sts_installer_trust_policy.jsonresources/sts/5.0/sts_installer_vpc_permission_boundary_policy.jsonresources/sts/5.0/sts_instance_controlplane_permission_policy.jsonresources/sts/5.0/sts_instance_controlplane_trust_policy.jsonresources/sts/5.0/sts_instance_worker_permission_policy.jsonresources/sts/5.0/sts_instance_worker_trust_policy.jsonresources/sts/5.0/sts_ocm_admin_permission_policy.jsonresources/sts/5.0/sts_ocm_permission_policy.jsonresources/sts/5.0/sts_ocm_trust_policy.jsonresources/sts/5.0/sts_ocm_user_trust_policy.jsonresources/sts/5.0/sts_support_permission_policy.jsonresources/sts/5.0/sts_support_trust_policy.jsonresources/wif/5.0/vanilla.yaml
Comment on lines
+29
to
+53
| "route53:ChangeResourceRecordSets", | ||
| "route53:ListHostedZonesByName", | ||
| "route53:ListResourceRecordSets" | ||
| ], | ||
| "Resource": "*" | ||
| }, | ||
| { | ||
| "Sid": "Route53ManageRecords", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "route53:ChangeResourceRecordSets" | ||
| ], | ||
| "Resource": "*", | ||
| "Condition": { | ||
| "ForAllValues:StringLike": { | ||
| "route53:ChangeResourceRecordSetsNormalizedRecordNames": [ | ||
| "*.openshiftapps.com", | ||
| "*.devshift.org", | ||
| "*.hypershift.local", | ||
| "*.openshiftusgov.com", | ||
| "*.devshiftusgov.com" | ||
| ] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Redundant route53:ChangeResourceRecordSets makes the conditional statement ineffective.
The action route53:ChangeResourceRecordSets is granted twice:
- Line 29: Unconditionally with
Resource: "*" - Lines 38-52: With domain restrictions via
Condition
Since IAM evaluates permissions with OR logic, the unconditional grant in the first statement allows modifying any Route53 record, completely bypassing the domain restrictions in the Route53ManageRecords statement. Either remove the action from line 29 or remove the second statement entirely if unrestricted access is intended.
Proposed fix: Remove duplicate action from first statement
"ec2:DescribeVpcEndpointServices",
"route53:ListHostedZonesByVPC",
"route53:ListTagsForResource",
"route53:GetHostedZone",
"route53:CreateHostedZone",
"route53:DeleteHostedZone",
"route53:ChangeTagsForResource",
- "route53:ChangeResourceRecordSets",
"route53:ListHostedZonesByName",
"route53:ListResourceRecordSets"
],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/sts/5.0/openshift_aws_vpce_operator_avo_aws_creds_policy.json`
around lines 29 - 53, The policy currently grants
route53:ChangeResourceRecordSets unconditionally and again in the statement with
Sid "Route53ManageRecords" (which includes domain restrictions), making the
conditional ineffective; remove "route53:ChangeResourceRecordSets" from the
unconditional Actions array (the earlier statement that also includes
"route53:ListHostedZonesByName" and "route53:ListResourceRecordSets") so that
only the "Route53ManageRecords" statement controls ChangeResourceRecordSets with
its Condition limiting domains.
Comment on lines
+35
to
+148
| { | ||
| "Sid": "AllowIAM", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "iam:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowELB", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "elasticloadbalancing:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowCloudWatch", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "cloudwatch:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowCloudWatchEvents", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "events:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowCloudWatchLogs", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "logs:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowSupport", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "support:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowKMS", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "kms:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowSTS", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "sts:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowTag", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "tag:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowRoute53", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "route53:*" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AllowServiceQuotas", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "servicequotas:ListServices", | ||
| "servicequotas:GetRequestedServiceQuotaChange", | ||
| "servicequotas:GetServiceQuota", | ||
| "servicequotas:RequestServiceQuotaIncrease", | ||
| "servicequotas:ListServiceQuotas" | ||
| ], | ||
| "Resource": [ | ||
| "*" | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate SCP policy files =="
fd -i 'osd_scp_policy.json'
echo
echo "== Find where this SCP is referenced/attached =="
rg -n --iglob '*.yaml' --iglob '*.yml' --iglob '*.json' \
'osd_scp_policy|ServiceControlPolicy|AWS::Organizations::Policy|organizations:CreatePolicy|organizations:AttachPolicy'
echo
echo "== Inspect deny-based guardrails in policy docs =="
rg -n --iglob '*.json' '"Effect"\s*:\s*"Deny"|\"iam:\*\"|\"sts:\*\"|\"kms:\*\"|\"ec2:\*\"'Repository: openshift/managed-cluster-config
Length of output: 815
Narrow overly broad wildcard SCP allows in resources/sts/5.0/osd_scp_policy.json
In resources/sts/5.0/osd_scp_policy.json (lines 35-148), the policy grants wide "Effect": "Allow" access using service-wide wildcard actions (e.g., iam:*, kms:*, sts:*, cloudwatch:*, events:*, logs:*, route53:*, tag:*, servicequotas:*) with Resource: "*". Scope to the minimal required actions/resources for sensitive services instead of using * service wildcards.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/sts/5.0/osd_scp_policy.json` around lines 35 - 148, The policy
currently uses overly broad wildcards (e.g., "iam:*", "kms:*", "sts:*",
"cloudwatch:*", "events:*", "logs:*", "route53:*", "tag:*", "servicequotas:*")
with Resource:"*", which violates least-privilege; update each statement (e.g.,
Sids: "AllowIAM", "AllowKMS", "AllowSTS", "AllowCloudWatch",
"AllowCloudWatchEvents", "AllowCloudWatchLogs", "AllowRoute53", "AllowTag",
"AllowServiceQuotas", "AllowELB", "AllowSupport") to enumerate only the specific
actions needed and narrow Resource ARNs (or use condition keys) instead of "*" —
for example replace "iam:*" with specific calls (ListRoles, PassRole,
CreateRole, etc.) scoped to required role ARNs, restrict "kms:*" to the specific
kms:Encrypt/Decrypt on the KMS key ARNs, limit "sts:*" to sts:AssumeRole with
specific role ARNs, and similarly scope CloudWatch/Logs/Events/Route53 actions
to the exact log groups, event rules, or hosted zones required; apply this
pattern across each Sid to enforce least-privilege.
2 tasks
Member
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
- Add resourceApplyMode: Sync to STS and WIF config.yaml files - Add FedRAMP exclusion selector to STS and WIF configs - Remove duplicate elasticloadbalancing:AddTags in kube-controller-manager policy - Remove duplicate logging.logEntries.list in WIF vanilla.yaml template - Regenerate hack templates via make
Contributor
|
@rosa-gap-analysis-bot: 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
|
@ravitri Can this PR please be updated to take account of changes merged in
thanks, Rob |
Member
|
@robpblake - Thanks Rob! Sure, will consider those PRs too and rebase |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Summary
This PR adds OpenShift 5.0.0-ec.1 credential policies and acknowledgments to address gap-analysis validation failures.
Prow Job: View Job Details
HTML Report: View Full Report
Baseline: 4.22.0-rc.4
Target: 5.0.0-ec.1
Validation Failures
The gap-analysis detected the following missing resources for OCP 5.0.0-ec.1:
Resolution
This PR adds the following files to resolve the validation failures:
AWS STS IAM Policies (30)
GCP Workload Identity Templates (1)
Acknowledgment Files (4)
Note: No OCP admin gates found for version 5.0 - no OCP acknowledgment files created.
Files Included
Total: 35 files
Note: Additional files (ACM policies, hack templates) are generated by the
makecommand after PR merge.Testing
All files have been:
Generated by ROSA Gap Analysis automation from Prow failure analysis.
Summary by CodeRabbit
New Features
Chores