[#98] Add a flag to redirect HTTP traffic to HTTPS #218

sestrella · 2022-11-27T21:33:44Z

what

Add a flag to redirect HTTP traffic to HTTPS
The hostname included in the redirection can be customized

why

It is a common best practice to redirect HTTP traffic to HTTPS
This workaround is necessary since Elastic Beanstalk HTTP listener rule can't be changed to a redirect action via the general options

references

Kudos to @yashshanker who posted the simplified solution
Closes Redirect http to https #98

joe-niland

Thanks @sestrella
Looks good. I have a couple of suggestions.
Let us know when it can be moved out of draft.

main.tf

sestrella · 2023-01-31T15:06:42Z

@joe-niland thank you for the review, I addressed most of the comments, however, I would like to do some testing on a project before flagging this PR as "Ready for Review"

sestrella · 2023-02-08T05:24:00Z

main.tf

+
+data "aws_lb_listener" "http" {
+  count             = local.enabled && var.loadbalancer_redirect_http_to_https ? 1 : 0
+  load_balancer_arn = var.loadbalancer_is_shared ? var.shared_loadbalancer_arn : one(aws_elastic_beanstalk_environment.default.0.load_balancers)


@joe-niland I tried to avoid hard-coding the 0 index here, however, it didn't work:

one(aws_elastic_beanstalk_environment.default.*.load_balancers)

On the other hand, I noticed the 0 index is hard-coded on the load_balancers output, so I'm using the same approach for now.

What was the error you saw with the above?

Sorry this was my mistake - in the example code I was thinking of something else and forgot the indexing was there just because count is used on the eb environment resource. This seems fine now.

sestrella · 2023-02-08T05:27:38Z

@joe-niland I did some local testing and it seems to work fine, this is the rule added to the HTTP listener:

joe-niland · 2023-02-08T05:55:42Z

/rebuild-readme

joe-niland · 2023-02-08T06:14:18Z

@sestrella would you mind adding this new var to https://github.com/cloudposse/terraform-aws-elastic-beanstalk-environment/tree/master/examples/complete so we can check it with Terratest?

sestrella · 2023-02-08T06:18:16Z

@joe-niland done

joe-niland · 2023-02-08T06:19:21Z

/test all

joe-niland · 2023-02-08T06:22:34Z

/rebuild-readme

joe-niland · 2023-02-08T06:23:01Z

@sestrella just a minor thing in the BATS test to look at

joe-niland · 2023-02-08T06:26:29Z

Also @sestrella could you please run the following:

make
make readme

joe-niland · 2023-02-08T06:27:04Z

@sestrella or change your PR to allow edits by maintainers: https://github.com/cloudposse/terraform-aws-elastic-beanstalk-environment/actions/runs/4121415576/jobs/7117081418

sestrella · 2023-02-08T06:52:29Z

@joe-niland I updated the README.md and docs/terraform.md files as suggested

joe-niland · 2023-02-08T06:57:54Z

/test all

joe-niland · 2023-02-08T10:11:45Z

@sestrella can you please update the version constraint in examples/**/versions.tf also?

joe-niland · 2023-02-08T22:10:37Z

/test bats

joe-niland · 2023-02-08T22:23:25Z

/test readme

joe-niland · 2023-02-08T22:52:36Z

/test terratest

joe-niland · 2023-02-08T22:53:37Z

@sestrella since you changed the main versions.tf again you'll need to regenerate the readme as well

sestrella · 2023-02-14T02:26:34Z

@joe-niland sorry for taking so long updating the README file, I got a weird error running make readme/build on Mac with M1:

make: *** [/Users/sestrella/code/stackbuilders/terraform-aws-elastic-beanstalk-environment/build-harness/modules/readme/Makefile:32: readme/build] Killed: 9

After taking a closer look, I realized that readme/deps step downloads the wrong binary for the arm64 architecture:

❯ uname -a
Darwin Administrators-MacBook-Pro 22.3.0 Darwin Kernel Version 22.3.0: Thu Jan  5 20:48:54 PST 2023; root:xnu-8792.81.2~2/RELEASE_ARM64_T6000 arm64 arm Darwin
…anstalk-environment on  redirect_http_to_https [?] via 💠 default via ❄️  impure (nix-shell)
➜ file build-harness/vendor/gomplate
build-harness/vendor/gomplate: Mach-O 64-bit x86_64 executable, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL>
…anstalk-environment on  redirect_http_to_https [?] via 💠 default via ❄️  impure (nix-shell)
➜ file build-harness/vendor/terraform-docs
build-harness/vendor/terraform-docs: Mach-O 64-bit x86_64 executable

Anyway, I found a workaround and I'm planning to report this issue to cloudposse/build-harness later.

README.md

joe-niland · 2023-02-14T19:17:49Z

/test all

joe-niland · 2023-02-14T19:49:30Z

Thanks for your contribution and patience @sestrella.
Released as https://github.com/cloudposse/terraform-aws-elastic-beanstalk-environment/releases/tag/0.49.0

Regarding the aws provider version constraint, was this updated by the make target?

sestrella · 2023-02-14T19:55:09Z

@joe-niland thank you for the code review and guidance! About:

Regarding the aws provider version constraint, was this updated by the make target?

I realized this version was coming from the .terraform.lock.hcl generated by terraform init so in the end I decided to delete this file and run make readme/build again which changed the constraint back to >= 3.0

joe-niland · 2023-02-14T20:05:35Z

I realized this version was coming from the .terraform.lock.hcl generated by terraform init so in the end I decided to delete this file and run make readme/build again which changed the constraint back to >= 3.0

Thanks for clarifying!

sestrella marked this pull request as ready for review November 27, 2022 22:08

sestrella requested review from a team as code owners November 27, 2022 22:08

sestrella requested review from jamengual and woz5999 November 27, 2022 22:08

sestrella marked this pull request as draft November 27, 2022 22:34

joe-niland reviewed Jan 31, 2023

View reviewed changes

main.tf Show resolved Hide resolved

main.tf Outdated Show resolved Hide resolved

main.tf Outdated Show resolved Hide resolved

main.tf Outdated Show resolved Hide resolved

sestrella added 9 commits January 31, 2023 00:11

Add flag to redirect HTTP to HTTPS

5ef41df

Rename variables and add support for shared load balancers

0147ce5

Modify existing examples

9090ad4

Update variables description

ad7a7c8

Fix redirect host default value

bf4484b

Replace custom status code with a flag

e24f26b

Fix format

ceae9c8

Fix bad reference to aws_elastic_beanstalk_environment

6eac554

Refactor datasources

06847ed

sestrella force-pushed the redirect_http_to_https branch from d1f2073 to 06847ed Compare January 31, 2023 05:30

sestrella added 3 commits February 7, 2023 14:24

Change status_code variable type

ef9c8a7

Fix load_balancer_arn reference

c370c19

Fix load_balancer_arn

7d8210b

sestrella marked this pull request as ready for review February 8, 2023 05:17

sestrella commented Feb 8, 2023

View reviewed changes

Add loadbalancer_redirect_http_to_https to complete example

f5e9c53

Add missing period at the end of the validation error message

4b6b921

Update README.md and docs/terraform.md

b64d8a8

sestrella added 2 commits February 8, 2023 16:50

Bump required_version

a4b755c

Update docs/terraform.md

6257984

Update README file

ce982ca

sestrella commented Feb 14, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

sestrella added 2 commits February 13, 2023 21:39

Change AWS provider constraint

2d34a08

Change AWS version constraint inside the README.md file

964ceed

joe-niland approved these changes Feb 14, 2023

View reviewed changes

joe-niland merged commit d4009cb into cloudposse:master Feb 14, 2023

sestrella deleted the redirect_http_to_https branch February 14, 2023 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#98] Add a flag to redirect HTTP traffic to HTTPS #218

[#98] Add a flag to redirect HTTP traffic to HTTPS #218

sestrella commented Nov 27, 2022 •

edited

Loading

joe-niland left a comment

sestrella commented Jan 31, 2023

sestrella Feb 8, 2023 •

edited

Loading

joe-niland Feb 8, 2023

sestrella Feb 8, 2023

joe-niland Feb 8, 2023

sestrella commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

sestrella commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

sestrella commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

sestrella commented Feb 14, 2023 •

edited

Loading

joe-niland commented Feb 14, 2023

joe-niland commented Feb 14, 2023

sestrella commented Feb 14, 2023

joe-niland commented Feb 14, 2023

[#98] Add a flag to redirect HTTP traffic to HTTPS #218

[#98] Add a flag to redirect HTTP traffic to HTTPS #218

Conversation

sestrella commented Nov 27, 2022 • edited Loading

what

why

references

joe-niland left a comment

Choose a reason for hiding this comment

sestrella commented Jan 31, 2023

sestrella Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

joe-niland Feb 8, 2023

Choose a reason for hiding this comment

sestrella Feb 8, 2023

Choose a reason for hiding this comment

joe-niland Feb 8, 2023

Choose a reason for hiding this comment

sestrella commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

sestrella commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

sestrella commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

joe-niland commented Feb 8, 2023

sestrella commented Feb 14, 2023 • edited Loading

joe-niland commented Feb 14, 2023

joe-niland commented Feb 14, 2023

sestrella commented Feb 14, 2023

joe-niland commented Feb 14, 2023

sestrella commented Nov 27, 2022 •

edited

Loading

sestrella Feb 8, 2023 •

edited

Loading

sestrella commented Feb 14, 2023 •

edited

Loading