Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[synthetics-private-location] Add service account annotations #1658

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kylemurphycambia
Copy link

What this PR does / why we need it:

This adds an optional annotations block for the synthetics-private-location service account, to use with AWS IAM roles, for example.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

  • fixes #

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Chart Version bumped
  • Documentation has been updated with helm-docs (run: .github/helm-docs.sh)
  • CHANGELOG.md has been updated
  • Variables are documented in the README.md

@kylemurphycambia kylemurphycambia requested a review from a team as a code owner January 4, 2025 04:04
Copy link
Contributor

@loutPhilipps loutPhilipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇 thank you for the change!

Edit: however, it looks like your commits aren't signed, which is something required for this repository. You can visit this page to setup commit signing, feel free to ask for another review once it's done!

@kylemurphycambia kylemurphycambia force-pushed the add_sa_annotations_synthetics branch from 8bcda2d to b985c33 Compare January 6, 2025 13:47
@kylemurphycambia kylemurphycambia force-pushed the add_sa_annotations_synthetics branch from b985c33 to 4489ba6 Compare January 6, 2025 13:56
@kylemurphycambia
Copy link
Author

🙇 thank you for the change!

Edit: however, it looks like your commits aren't signed, which is something required for this repository. You can visit this page to setup commit signing, feel free to ask for another review once it's done!

Sorry I missed this, finally figured it out

@kylemurphycambia
Copy link
Author

@loutPhilipps - thanks for the approval!

Looks like the labeler doesn't like that this is coming from a forked repo. Should I recreate this PR another way? Or is there a workaround on your side to resolve this failed check?

@loutPhilipps
Copy link
Contributor

@loutPhilipps - thanks for the approval!

Looks like the labeler doesn't like that this is coming from a forked repo. Should I recreate this PR another way? Or is there a workaround on your side to resolve this failed check?

Looks like there is a repo issue with this labeler, I think you can just merge with the failing check

@kylemurphycambia
Copy link
Author

kylemurphycambia commented Jan 6, 2025

/merge

edit: Sorry to keep bothering you @loutPhilipps - someone with more authority needs to merge

@kylemurphycambia
Copy link
Author

@rafamelo12 / @L3n41c - any chance one of you can merge this for me? I don't have permissions to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants