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

Implemented #390 Tags support on IAMbic hub and spoke role creation #392

Merged
merged 12 commits into from
May 9, 2023

Conversation

smoy
Copy link
Contributor

@smoy smoy commented May 5, 2023

What changed?

  • Tags support on cloudformation stack and stack sets creation. The tags propagate to resources created by stack and stack sets.
  • Support custom name for IambicHubRole
  • Support custom name for IambicSpokeRole

Rationale

  • Rather than plumbing specific tag support on just IAM roles. It seems more convention to attach tags on the stack and stack sets, and let cloudformation to propagate tags down to any resources created.
  • Support custom name for IambicHubRole and IambicSpokeRole helps a lot during development and testing because I don't have to blow away existing installation. (so there is no role creation conflict)

How was it tested?

If it was manually verified, list the instructions for your reviewers to follow.

  • Unit Tests
  • Functional Tests
  • Manually Verified

Run iambic setup, and iambic import

@smoy smoy self-assigned this May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Patch coverage: 75.55% and project coverage change: -10.01 ⚠️

Comparison is base (ebcd894) 85.28% compared to head (fe9ba2d) 75.27%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #392       +/-   ##
===========================================
- Coverage   85.28%   75.27%   -10.01%     
===========================================
  Files          98       98               
  Lines       10612    10648       +36     
===========================================
- Hits         9050     8015     -1035     
- Misses       1562     2633     +1071     
Flag Coverage Δ
functional_tests ?
functional_tests_config_discovery ?
unit_tests 75.27% <75.55%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
iambic/plugins/v0_1_0/aws/cloud_formation/utils.py 16.54% <10.00%> (-0.92%) ⬇️
iambic/main.py 51.01% <66.66%> (+0.24%) ⬆️
iambic/config/utils.py 88.00% <95.65%> (+6.51%) ⬆️
iambic/plugins/v0_1_0/aws/models.py 59.29% <100.00%> (-26.77%) ⬇️

... and 45 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.


def parse_pair(s) -> dict:
key, value = s.rstrip().split("=")
return {"Key": key, "Value": value}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to do some regex validation on tags? And char limit check? (I feel like we have this type of function somewhere in cloudumi but I can't find it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added validate callback in questionnaire. I check aws-cli doesn't actually check on the cli level, and defer regex check on boto3.

@Will-NOQ
Copy link
Collaborator

Will-NOQ commented May 9, 2023

Tests are failing 😞

@smoy smoy requested a review from castrapel May 9, 2023 18:01
castrapel
castrapel previously approved these changes May 9, 2023
Copy link
Contributor

@castrapel castrapel left a comment

Choose a reason for hiding this comment

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

LGTM!

CHANGELOG.md Outdated

THANKS:

* `noq@phad.me`, `perpil` in [NoqCommunity](https://noqcommunity.slack.com/archives/C02P9E8BDK6/p1683275443604049) proposing tags support during IAMbic setup.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is noq@phad.me formatted properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gonna use consistent First Name, Last initial since the conversation is from slack. (avoiding mixing Github identities and Slack)

castrapel
castrapel previously approved these changes May 9, 2023
@smoy smoy merged commit 9d14fe8 into main May 9, 2023
3 of 5 checks passed
@smoy smoy deleted the feature/390-hub-spoke-role-tags branch May 9, 2023 22:54
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.

None yet

3 participants