-
Notifications
You must be signed in to change notification settings - Fork 75
RUM-11897: Add scripts to set/get Vault secrets & Migrate CI #2974
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
base: develop
Are you sure you want to change the base?
Conversation
5e9a81e to
5bde49e
Compare
|
🎯 Code Coverage 🔗 Commit SHA: 85c6bfd | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2974 +/- ##
===========================================
- Coverage 70.91% 70.84% -0.07%
===========================================
Files 829 829
Lines 30381 30381
Branches 5184 5184
===========================================
- Hits 21543 21523 -20
- Misses 7373 7378 +5
- Partials 1465 1480 +15 🚀 New features to boost your workflow:
|
c83a82e to
b166c1b
Compare
c096dd5 to
bc86c66
Compare
bc86c66 to
85c6bfd
Compare
|
|
||
| fetch-secrets: | ||
| stage: fetch-secrets | ||
| tags: ["macos:sonoma","specific:true"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a benefit of using macOS runner for that?
Also if it indeed should be macOS runner, then image directive below has no effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an obligation, only MacOS runner has the correct IAM role to access Vault of our project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only MacOS runner has the correct IAM role
Why so? AFAIK every repo has a unique service account associated, so maybe we can list it as well as account which can have this role?
The fleet of macOS runners is much limited in size, while k8s runners are faster to get and run.
Not a blocker though.
| artifacts: | ||
| paths: | ||
| - ./ci/pipelines/secrets/ | ||
| expire_in: 1 hour | ||
| when: always |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a safe thing to have? it means that anyone with just a basic Gitlab CI access can download this, not only RUM team members.
Moreover, it makes this artifact available for 1 hour, meaning it is still there even after pipeline completed, failed.
Should we maybe fetch secrets only in the boundaries of the particular job?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the default behavior will be only uploading artifacts when success, do you think remove when:always will improve this?
fetching secrets in each job may cause repeat vault login and vault get, it may increase the CI time, I can ask around if this is a safe way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetching secrets in each job may cause repeat vault login and vault get
It should be fast. Anyway, by constraining secrets lifetime only to the particular job and removing the possibility to fetch them as an artifact we limit access only to the Repo Admins/Contributors (it is still possible to modify CI script to read them) compared to the Job artifacts access which is possible for everyone with dev access.
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)