Fix: Add annotations regarding s3 related information for serviceaccount#391
Conversation
Some test casess use serviceaccount to set aws credential but the way how to set endpoint is wrong. It should use annotations. Signed-off-by: Jooho Lee <jlee@redhat.com>
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/cherry-pick', '/wip', '/lgtm', '/hold', '/verified', '/build-push-pr-image'} |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe context manager responsible for creating S3 endpoint Secret resources was updated to include additional annotations. These new annotations provide details such as the S3 endpoint (without protocol), region, credential usage, SSL verification, and whether HTTPS is used, enriching the metadata associated with the Secret. Changes
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
utilities/infra.py (2)
322-325: HTTPS detection logic is correct.The logic correctly determines HTTPS usage by checking the endpoint protocol prefix. The implementation is sound and addresses the S3 configuration requirements.
Consider using boolean values and string literals for improved readability:
- # Determine usehttps based on endpoint protocol - usehttps = 0 - if aws_s3_endpoint.startswith("https://"): - usehttps = 1 + # Determine usehttps based on endpoint protocol + usehttps = "1" if aws_s3_endpoint.startswith("https://") else "0"
327-334: Enhanced S3 annotations provide comprehensive configuration metadata.The new annotations correctly address the S3 endpoint configuration issues mentioned in the PR objectives. The endpoint processing properly removes protocol prefixes, and the metadata provides all necessary S3 connection parameters.
Note that
s3-useanoncredentialands3-verifysslare hardcoded, which appears appropriate for the test environment context but may limit flexibility for other use cases.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utilities/infra.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
utilities/infra.py (1)
utilities/constants.py (1)
ApiGroups(122-124)
|
/lgtm |
|
Status of building tag latest: success. |
Description
Some test cases configure AWS credentials using a service account, but the S3 endpoint is set incorrectly — it should be provided via annotations.
How Has This Been Tested?
Merge criteria: