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

Move IAM Service Account Tasks to actions package #7140

Closed
wants to merge 6 commits into from

Conversation

TiberiuGC
Copy link
Collaborator

@TiberiuGC TiberiuGC commented Oct 5, 2023

Description

Closes #6961

Migrates IAM service account create/delete tasks to the actions/irsa package, reducing the complexity of StackManager interface used for defining interactions with CFN API.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@TiberiuGC TiberiuGC added the area/tech-debt Leftover improvements in code, testing and building label Oct 5, 2023
@TiberiuGC TiberiuGC force-pushed the maintenance/move-iam-sa-related-tasks branch from d3fd457 to 5dc91f9 Compare October 9, 2023 12:08
@TiberiuGC TiberiuGC force-pushed the maintenance/move-iam-sa-related-tasks branch from 5dc91f9 to e10ecfe Compare October 10, 2023 07:13
@TiberiuGC TiberiuGC marked this pull request as ready for review October 10, 2023 08:54
@TiberiuGC TiberiuGC requested a review from a-hilaly October 17, 2023 06:39
errMsg := fmt.Sprintf("deleting addon IAM %q", *t.stack.StackName)
if t.wait {
if err := t.stackManager.DeleteStackBySpecSync(t.ctx, t.stack, errorCh); err != nil {
return fmt.Errorf("%s: %w", errMsg, err)
}
return nil
}

defer close(errorCh)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Closing error channel to early here was nullifying the effect of the --wait flag

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It actually introduced a bug - #7177. Going to address it in a separate PR.

@github-actions github-actions bot added the stale label Nov 23, 2023
@Himangini Himangini removed the stale label Nov 27, 2023
@github-actions github-actions bot added the stale label Dec 28, 2023
@github-actions github-actions bot closed this Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tech-debt Leftover improvements in code, testing and building stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Move IAM Service Accounts related tasks to actions package
2 participants