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

Small fixes #36

Closed
wants to merge 2 commits into from
Closed

Small fixes #36

wants to merge 2 commits into from

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Jul 29, 2022

what

  • Fix empty target_security_group_id

why

  • If this is empty, it will throw an error
│ Error: Invalid index
│
│   on .terraform/modules/sftp.security_group/main.tf line 36, in locals:36:   cbd_security_group_id = local.create_security_group ? one(aws_security_group.cbd[*].id) : var.target_security_group_id[0]
│     ├────────────────
│     │ var.target_security_group_id is empty list of string

references

  • N/A

@nitrocode nitrocode changed the title Fix empty target_security_group_id Small fixes Jul 29, 2022
@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode marked this pull request as ready for review July 29, 2022 01:22
@nitrocode nitrocode requested review from a team as code owners July 29, 2022 01:22
@nitrocode nitrocode requested review from Gowiem, florian0410 and Nuru July 29, 2022 01:22
@Nuru Nuru added invalid This doesn't seem right wontfix This will not be worked on labels Jul 29, 2022
@Nuru
Copy link
Contributor

Nuru commented Jul 29, 2022

If local.create_security_group is false then this module is not creating a security group and it is therefore an error to not supply a target_security_group_id. This is intentional.

@Nuru Nuru closed this Jul 29, 2022
@nitrocode
Copy link
Member Author

What variable in the sg export should be mapped to the target_security_group_id ?

@Nuru
Copy link
Contributor

Nuru commented Sep 9, 2022

What variable in the sg export should be mapped to the target_security_group_id ?

None. Other Cloud Posse modules using this module should, in general, either create a security group or accept an already configured security group. If another module were to accept a security group ID and then use this module to modify the security group, this would likely cause flapping between the configuration of the security group in that module and the configuration in whatever module created the security group.

Every Cloud Posse module should output enough information for the user to configure any security group they want to work with the resources created by that module. If we find a use case where it is appropriate for the module to modify the security group it is given, we can address that as a special case.

@nitrocode nitrocode deleted the fix-empty-target_security_group_id branch September 11, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants