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

Support multiple baselines on SSM Patch Group #13752

Conversation

melbit-michaelw
Copy link
Contributor

This commit modifies the Id for an SSM Patch Group to be the Patch Group Name joined to the BaselineId by a ":". This allows an SSM Patch Group to support multiple baselines.

Please note that pre-existing patch groups will be recreated due to the change in Id (this happens without issue in my testing as the AWS API call succeeds even if the patch group already has that baseline applied).

If necessary, I can look into State Migration to handle that, but it looked quite complicated compared to my very beginner level of Go knowledge.

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #9603

Release note for CHANGELOG:

resource/aws_ssm_patch_group: Support multiple baselines (one per OS) on an SSM patch group 

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSSMPatchGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSSMPatchGroup -timeout 120m
=== RUN   TestAccAWSSSMPatchGroup_basic
=== PAUSE TestAccAWSSSMPatchGroup_basic
=== CONT  TestAccAWSSSMPatchGroup_basic
--- PASS: TestAccAWSSSMPatchGroup_basic (29.14s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       30.406s

This commit modifies the Id for a patch group to be the Patch Group Name joined to the BaselineId by a ":".
@melbit-michaelw melbit-michaelw requested a review from a team June 15, 2020 04:22
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/ssm Issues and PRs that pertain to the ssm service. needs-triage Waiting for first response or review from a maintainer. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jun 15, 2020
Copy link
Contributor

@jdheyburn jdheyburn left a comment

Choose a reason for hiding this comment

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

This looks good 👍 . I'll try to clone your repo and give it a go later this week. In the meantime I've some comments on readability

aws/resource_aws_ssm_patch_group.go Show resolved Hide resolved
aws/resource_aws_ssm_patch_group_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jdheyburn jdheyburn left a comment

Choose a reason for hiding this comment

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

I've built the provider locally and confirmed the fix given the replicate steps in the linked issue. Happy for it to be merge.

Thanks for getting round to looking at this 🎉

@melbit-michaelw
Copy link
Contributor Author

@bflad - Apologies for mentioning you directly, but this pull request appears to have been overlooked. Is there anything I can do in order to get it reviewed and hopefully merged ?

@melbit-michaelw
Copy link
Contributor Author

@anGie44 , @YakDriver - Again, really sorry to mention you directly, but this PR hasn't progressed in months. Is there something more I need to do in order for it to be reviewed ?

@anGie44
Copy link
Contributor

anGie44 commented Mar 17, 2021

Hi @melbit-michaelw , thank you for raising this PR! Since these changes are included in #15213, I'm going to close this in favor of the other PR which includes pagination support. Please refer there for updates 👍

@anGie44 anGie44 closed this Mar 17, 2021
@anGie44 anGie44 removed the needs-triage Waiting for first response or review from a maintainer. label Mar 17, 2021
@ghost
Copy link

ghost commented Apr 16, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/ssm Issues and PRs that pertain to the ssm service. size/XS Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resource aws_ssm_patch_group cannot handle multiple baselines of differing OS in a single patch group
3 participants