WIP: OCPBUGS-96892: move snapshot actions to unscoped

[rhrmo]

Summary by CodeRabbit

• Bug Fixes
  • Updated AWS permission handling so EC2 snapshot creation and deletion actions are no longer treated as tag-scoped.
  • Improved the accuracy of tag-condition checks for snapshot-related IAM actions.

[openshift-ci-robot]

@rhrmo: This pull request references Jira Issue OCPBUGS-96892, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In 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.

[coderabbitai]

Walkthrough

Reclassifies ec2:CreateSnapshot and ec2:DeleteSnapshot from aws:ResourceTag-scoped actions to unscoped actions in the IAM utility logic, changing the result of SupportsInfraResourceTagCondition for these actions, and updates corresponding test expectations.

Changes

EC2 snapshot action reclassification

Layer / File(s)	Summary
Reclassify snapshot actions as unscoped pkg/aws/utils.go	ec2:CreateSnapshot and ec2:DeleteSnapshot removed from infraResourceTagScopedActions and added to infraResourceTagUnscopedActions.
Update test expectations pkg/aws/utils_test.go	payloadAWSActions test data updated so these two actions expect false (unscoped) instead of true.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: moving snapshot actions to an unscoped configuration.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	No Ginkgo tests or unstable titles were added; the Go subtests use static strings or fixed action literals, so names are deterministic.
Test Structure And Quality	✅ Passed	PASS: The PR only updates a table-driven unit test and a classification map; no Ginkgo, cluster resources, Eventually/Consistently, or cleanup concerns are introduced.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; pkg/aws/utils_test.go is a plain Go unit test with no MicroShift-unsupported APIs.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; only AWS utility logic and a unit test changed, so no SNO-specific multi-node assumptions are introduced.
Topology-Aware Scheduling Compatibility	✅ Passed	Only AWS IAM action maps/tests changed; no manifests, controllers, or scheduling constraints were added.
Ote Binary Stdout Contract	✅ Passed	Only AWS action classification/tests changed; no main/TestMain/suite setup or stdout writes were added.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests were added; only unit tests and AWS action map updates in pkg/aws, with no IPv4 assumptions or external connectivity.
No-Weak-Crypto	✅ Passed	Touched code only updates AWS IAM action scoping maps/tests; no crypto primitives, weak algorithms, or secret comparisons appear in the diff.
Container-Privileges	✅ Passed	Only pkg/aws Go files changed; no container/K8s manifests or privilege settings were introduced.
No-Sensitive-Data-In-Logs	✅ Passed	Patch only changes IAM action classification and tests; no new or modified logging of secrets, tokens, PII, hostnames, or customer data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rhrmo
Once this PR has been reviewed and has the lgtm label, please assign jstuever for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@pkg/aws/utils.go`:
- Around line 177-179: The snapshot permission grouping in the AWS utilities is
too broad: `ec2:DeleteSnapshot` must remain in the scoped permissions path
because it supports `aws:ResourceTag` and should keep the cluster-ownership
guard. Update the permission map in `utils.go` so `ec2:DeleteSnapshot` is
handled separately from the unscoped snapshot bucket, and only adjust
`ec2:CreateSnapshot` independently if needed; use the existing AWS permission
grouping around the snapshot entries to locate the fix.

🪄 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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8982687e-9b57-474d-879a-77b38492cae5

📥 Commits

Reviewing files that changed from the base of the PR and between 0564adf and c0a23d4.

📒 Files selected for processing (2)

• pkg/aws/utils.go
• pkg/aws/utils_test.go

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant file and nearby helpers.
git ls-files pkg/aws/utils.go
wc -l pkg/aws/utils.go
sed -n '130,240p' pkg/aws/utils.go

# Find where these maps are consumed.
rg -n "infraResourceTagUnscopedActions|CreateSnapshot|DeleteSnapshot|SupportsInfraResourceTagCondition|ResourceTag" pkg/aws

Repository: openshift/cloud-credential-operator

Length of output: 10905

---

pkg/aws/utils.go:177-179 — Keep ec2:DeleteSnapshot scoped. ec2:DeleteSnapshot supports aws:ResourceTag, so moving it into the unscoped bucket drops the cluster-ownership guard for snapshot deletion. ec2:CreateSnapshot may need separate handling, but it shouldn’t force deletion into the same bucket.

🤖 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 `@pkg/aws/utils.go` around lines 177 - 179, The snapshot permission grouping in
the AWS utilities is too broad: `ec2:DeleteSnapshot` must remain in the scoped
permissions path because it supports `aws:ResourceTag` and should keep the
cluster-ownership guard. Update the permission map in `utils.go` so
`ec2:DeleteSnapshot` is handled separately from the unscoped snapshot bucket,
and only adjust `ec2:CreateSnapshot` independently if needed; use the existing
AWS permission grouping around the snapshot entries to locate the fix.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.28%. Comparing base (0564adf) to head (c0a23d4).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1054   +/-   ##
=======================================
  Coverage   47.28%   47.28%           
=======================================
  Files          97       97           
  Lines       12631    12631           
=======================================
  Hits         5973     5973           
  Misses       6001     6001           
  Partials      657      657           

Files with missing lines	Coverage Δ
pkg/aws/utils.go	86.11% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci]

PR needs rebase.

Details

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-ci]

@rhrmo: 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/security	c0a23d4	link	true	/test security

Full PR test history. Your PR dashboard.

Details

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.