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

Minor typo on S3 bucket key prefix #39

Closed
wants to merge 2 commits into from
Closed

Minor typo on S3 bucket key prefix #39

wants to merge 2 commits into from

Conversation

frite
Copy link

@frite frite commented Dec 14, 2023

what

This PR fixes a minor typo on main.tf

why

The key prefix of the S3 bucket was reading scaning. I believe it should read scanning.

references

@frite frite requested review from a team as code owners December 14, 2023 11:07
@frite frite requested review from jamengual and nitrocode December 14, 2023 11:07
@frite
Copy link
Author

frite commented Jan 17, 2024

Hey @jamengual & @nitrocode any updates??

@frite
Copy link
Author

frite commented Jan 23, 2024

Hi @nitrocode and @jamengual, any updates on this?

@nitrocode nitrocode removed their request for review January 24, 2024 01:25
@jamengual
Copy link
Contributor

/terratest

@jamengual
Copy link
Contributor

@frite could you run make precommit/terraform and commit back the changes please?

@frite
Copy link
Author

frite commented Feb 15, 2024

Hi @jamengual, this is probably not what you asked for but...

$ make precommit/terraform
Starting cloudposse/build-harness:slim-latest
docker run --name build-harness \
		--rm -it \
		--platform linux/amd64 \
		-e PACKAGES_PREFER_HOST=true \
		-e AWS_ACCESS_KEY_ID -e AWS_SECRET_ACCESS_KEY -e AWS_SESSION_TOKEN -e TERM -e AWS_PROFILE -e AWS_REGION -e AWS_DEFAULT_PROFILE -e AWS_DEFAULT_REGION -e AWS_CONFIG_FILE -e AWS_SHARED_CREDENTIALS_FILE \
		 \
		-v code/terraform-aws-ssm-patch-manager:/host \
		--workdir /host \
		--entrypoint /usr/bin/make \
		cloudposse/build-harness:slim-latest terraform/precommit
tflint --enable-plugin=aws
1 issue(s) found:

Warning: [Fixable] variable "region" is declared but not used (terraform_unused_declarations)

  on variables.tf line 1:
   1: variable "region" {

Reference: https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.5.0/docs/rules/terraform_unused_declarations.md

make: *** [/build-harness/modules/terraform/Makefile:35: terraform/tflint] Error 2
make: *** [build-harness/runner] Error 2

In a nutshell I get an error on tflint for not using the region variable which is not part of this PR. How would you like me to proceed forward?

This PR contains the output of `make precommit/terraform` as part of the
original PR. Additionally, `tflint` was flagging `region` variable as
unused. The proper directive was added to suppress that warning since
the variable can be set by the user.
@frite
Copy link
Author

frite commented Feb 15, 2024

So @jamengual, I decided to add a directive to suppress the tflint warning. The region variable is used (e.g., in https://github.com/cloudposse/terraform-aws-ssm-patch-manager/blob/main/examples/complete/main.tf#L49). I've also included the produced README.md file following the successful output of make precommit/terraform.

Let me know what you think.

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.

2 participants