Skip to content

feat: Add cloudwatch and security group outputs #121

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

Merged
merged 1 commit into from
Apr 16, 2025

Conversation

adamcrews
Copy link
Contributor

@adamcrews adamcrews commented Mar 26, 2025

what

This adds the outputs for the optionally created cloudwatch logs and the security group.

why

In my use case, this information is useful to pass on to other modules, thus adding the outputs helps expose that information.

This also supersedes PR #116
and closes issue #91

@adamcrews adamcrews requested review from a team as code owners March 26, 2025 23:29
@mergify mergify bot added the triage Needs triage label Mar 26, 2025
@adamcrews adamcrews force-pushed the feature/additional_outputs branch from 675c0b1 to 73a844a Compare March 26, 2025 23:31
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

Small changes. Running tests to confirm all is good 👍

@Gowiem
Copy link
Member

Gowiem commented Mar 27, 2025

/terratest

@adamcrews adamcrews force-pushed the feature/additional_outputs branch from 73a844a to 2a9130d Compare March 27, 2025 18:58
@adamcrews adamcrews requested a review from Gowiem March 27, 2025 18:58
@adamcrews adamcrews force-pushed the feature/additional_outputs branch from 2a9130d to 72cd5f2 Compare March 27, 2025 19:34
@Gowiem Gowiem added enhancement New feature or request minor New features that do not break anything feature New functionality labels Apr 10, 2025
@Gowiem
Copy link
Member

Gowiem commented Apr 10, 2025

/terratest

@Gowiem
Copy link
Member

Gowiem commented Apr 14, 2025

@adamcrews this is annoying, but can you add descriptions to all of the outputs in examples/complete? Appreciate it.

Test: check if terraform outputs have descriptions
File: output-descriptions.bats
---------------------------------
client_configuration is missing a description
vpn_endpoint_arn is missing a description
vpn_endpoint_dns_name is missing a description
vpn_endpoint_id is missing a description
---------------------------------

not ok 1 check if terraform outputs have descriptions
# (in test file /test/terraform/output-descriptions.bats, line 16)
#   `[ -z "$output" ]' failed
+ status=fail
+ [[ fail == \p\a\s\s ]]
+ pass=false
+ printf '\n\nEnd of testing in /__w/terraform-aws-ec2-client-vpn/terraform-aws-ec2-client-vpn/examples/complete\n\n\n'
+ [[ false == \t\r\u\e ]]


End of testing in /__w/terraform-aws-ec2-client-vpn/terraform-aws-ec2-client-vpn/examples/complete

@adamcrews adamcrews force-pushed the feature/additional_outputs branch from 72cd5f2 to 20af392 Compare April 15, 2025 17:07
Copy link

mergify bot commented Apr 15, 2025

💥 This pull request now has conflicts. Could you fix it @adamcrews? 🙏

@mergify mergify bot added the conflict This PR has conflicts label Apr 15, 2025
@adamcrews adamcrews force-pushed the feature/additional_outputs branch from 20af392 to 69b97f6 Compare April 15, 2025 17:16
@adamcrews
Copy link
Contributor Author

@adamcrews this is annoying, but can you add descriptions to all of the outputs in examples/complete? Appreciate it.

No worries. It looks like these failures pre-date my changes, but I added descriptions to the existing outputs, and added all outputs to the example/complete code.

@mergify mergify bot removed the conflict This PR has conflicts label Apr 15, 2025
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

LGTM

@Gowiem Gowiem enabled auto-merge (squash) April 15, 2025 18:07
@mergify mergify bot removed the triage Needs triage label Apr 15, 2025
@Gowiem
Copy link
Member

Gowiem commented Apr 16, 2025

/terratest

@Gowiem Gowiem merged commit 1d34333 into cloudposse:main Apr 16, 2025
37 checks passed
Copy link

These changes were released in v1.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature New functionality minor New features that do not break anything
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants