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.

Sign up for GitHub

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

Jump to bottom

cli: add iam upgrade apply #2132

Merged
merged 16 commits into from
Jul 26, 2023
Merged

cli: add iam upgrade apply #2132

merged 16 commits into from
Jul 26, 2023

Conversation

elchead
Copy link
Contributor

@elchead elchead commented Jul 25, 2023 •
edited
Loading

Context

#2090 introduced a new AWS LB controller which requires additional IAM permissions.
In order to keep IAM and cluster management separate, the IAM permissions should be upgraded separately.

Proposed change(s)

  • iam upgrade apply
  • all PRs requiring IAM upgrades should have a label iam upgrade to streamline the release process (see updated process). This allows to issue prompts to the user on upgrade apply

Checklist

  • Update docs
  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@netlify
Copy link

netlify bot commented Jul 25, 2023 •
edited
Loading

✅ Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit c46854f
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/64c13b88ec448f000808669d

@elchead elchead requested review from derpsteb and msanft July 25, 2023 09:43
@elchead elchead requested a review from thomasten as a code owner July 25, 2023 10:06
@derpsteb
Copy link
Member

derpsteb commented Jul 25, 2023 •
edited
Loading

Code lgtm, will quickly do a manual test.

I think it would make sense to add a section to .github/release.yml that includes the docs link. That section would be added to the changelog in case a PR with label "iam upgrade" is found. wdyt?

derpsteb
derpsteb approved these changes Jul 25, 2023
View reviewed changes
Copy link
Member

@derpsteb derpsteb left a comment

Choose a reason for hiding this comment

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

Manual test worked fine.
I think we should integrate this into the upgrade e2e workflow. Or add a separate test for it.

@derpsteb derpsteb mentioned this pull request Jul 25, 2023
Merged
2 tasks
@elchead
Copy link
Contributor Author

elchead commented Jul 26, 2023 •
edited
Loading

Manual test worked fine. I think we should integrate this into the upgrade e2e workflow. Or add a separate test for it.

I suggest either :

EDIT: option 2 run: https://github.com/edgelesssys/constellation/actions/runs/5665254477

@elchead elchead force-pushed the feat/iam/upgrade-apply branch 2 times, most recently from e9c0c89 to a9224e1 Compare July 26, 2023 09:36
@elchead elchead requested a review from derpsteb July 26, 2023 12:22
@elchead
Copy link
Contributor Author

elchead commented Jul 26, 2023 •
edited
Loading

waiting for e2e upgrade to complete: https://github.com/edgelesssys/constellation/actions/runs/5667921845/job/15359955050 gcp ✅
https://github.com/edgelesssys/constellation/actions/runs/5669525126 aws ✅

@elchead elchead merged commit a3184af into main Jul 26, 2023
5 of 6 checks passed
@elchead elchead deleted the feat/iam/upgrade-apply branch July 26, 2023 15:29
This was referenced Aug 8, 2023
Merged
Merged
@elchead elchead added the feature This introduces new functionality label Aug 14, 2023
@elchead elchead added this to the v2.10.0 milestone Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derpsteb derpsteb derpsteb approved these changes

@msanft msanft Awaiting requested review from msanft

@thomasten thomasten Awaiting requested review from thomasten thomasten is a code owner

Assignees
No one assigned
Labels
feature This introduces new functionality
Projects
None yet
Milestone
v2.10.0
Development

Successfully merging this pull request may close these issues.
2 participants
@elchead @derpsteb