Making CIDR Blocks Configurable #85

MatthiasScholz · 2021-01-03T23:26:22Z

Solves #84.

Refactoring the terraform configuration to configure:

SSH access using CIDR blocks
Traffic access using CIDR blocks

Additionally a test is provided testing SSH access for access and deny.
Unfortunately, due to a bug in the terraform CLI test execution, with positive and negative access testing, was not possible using MacOSX operating system and hence only a positive test using the existing example is provided.

Testing SSH access depending on the CIDR block configuration. Nevertheless due to a bug in the terraform CLI: - hashicorp/terraform#17032 test execution is not possible until the bug is solved.

hashicorp-cla · 2021-01-03T23:26:25Z

All committers have signed the CLA.

brikis98

Thanks for the PR! Please see my comment about examples vs prod code.

I think the part of the PR that's useful are the changes to the modules folder to make the outbound CIDR blocks configurable, and to not create inbound SSH SG rules if that list is empty. Everything else seems to affect examples only and is intentionally left open.

examples/nomad-consul-separate-cluster/main.tf

modules/nomad-cluster/main.tf

modules/nomad-cluster/variables.tf

test/README.md

Co-authored-by: Yevgeniy Brikman <brikis98@users.noreply.github.com>

examples/nomad-consul-separate-cluster/main.tf

test/nomad_cluster_ssh_test.go

Reverting the changes made to allow the configuration of the CIDR blocks for SSH and outbound traffic for the examples. Additionally, since the examples now lacking the flexibility, the SSH test now only checks if access is possible.

brikis98

OK, one more NIT, and I can kick off tests!

brikis98 · 2021-01-14T11:12:41Z

test/nomad_helpers.go

+		aws.DeleteAmi(t, awsRegion, amiId)
+	})
+
+		test_structure.RunTestStage(t, "setup_ami", func() {


Could you run go fmt on this code? The indentation looks off.

Ah, I see - thanks for pointing this out. It should be fixed now.

Waiting for PR to be merge: hashicorp/terraform-aws-nomad#85

* 📌 version updates for all major components | #85 Covering the following dependencies: - nomad: 1.0.2 - consul: 1.9.1 - fabio - terraform modules - terraform-aws-consul module version 0.8.2 - packer definition - terraform: 0.14.4 * ♻️ Migrate from SSH to SSM and restructuring | #83 - Remove SSH dependency - Using AWS System Manager Session Manager instead of SSH access. * ✨ AMI testing during creation - Checking the AMI during the packer build step using goss. * ✨ restart nomad service on instance, solves #62 * 👷 added linting * 🐛 fixing aws provider version due to autoscaling issue An unsolved regression in the terraform-provider-aws ( hashicorp/terraform-provider-aws#14085 ) prevents the creation of autoscaling groups using terraform. * 💩 using fork to unblock waiting for PR | #85 Waiting for PR to be merge: hashicorp/terraform-aws-nomad#85 Co-authored-by: Matthias Scholz <matthias.scholz@gmail.com> Co-authored-by: Matthias Scholz <matthias.scholz@thoughtworks.com>

brikis98

Thanks! I'll kick off tests now.

brikis98 · 2021-01-18T18:32:57Z

Looks like you still need a go fmt:

[INFO] Initializing environment for https://github.com/gruntwork-io/pre-commit.
Terraform fmt............................................................Passed
gofmt....................................................................Failed
- hook id: gofmt
- files were modified by this hook

test/nomad_helpers.go
test/nomad_helpers.go

brikis98 · 2021-01-19T08:59:06Z

Running tests once more!

brikis98 · 2021-01-19T09:05:29Z

Looks like you're missing some imports in the test code:

go: downloading github.com/russross/blackfriday/v2 v2.0.1
go: downloading github.com/shurcooL/sanitized_anchor_name v1.0.0
# github.com/gruntwork-io/terraform-aws-nomad/test [github.com/gruntwork-io/terraform-aws-nomad/test.test]
./nomad_helpers.go:57:17: undefined: test_structure
./nomad_helpers.go:59:8: undefined: test_structure
./nomad_helpers.go:60:23: undefined: test_structure
./nomad_helpers.go:63:12: undefined: test_structure
./nomad_helpers.go:64:16: undefined: test_structure
./nomad_helpers.go:68:2: undefined: test_structure
./nomad_helpers.go:70:3: undefined: test_structure
./nomad_helpers.go:73:3: undefined: test_structure
./nomad_helpers.go:76:3: undefined: test_structure
./nomad_helpers.go:79:2: undefined: test_structure
./nomad_helpers.go:79:2: too many errors
FAIL	github.com/gruntwork-io/terraform-aws-nomad/test [build failed]

Did you ever run or at least compile this code?

Add support for terraform >= 0.14.3 using a wrapper function to bypass terratest limitation: - gruntwork-io/terratest#766 Adding missing SSH key pair to make SSH test work.

brikis98 · 2021-01-22T12:54:41Z

Running tests again!

MatthiasScholz · 2021-01-22T21:55:34Z

Since I ran into the issue with an incompatible terraform / terratest combination, should I update the referenced terratest version as well and remove the workaround - or should the terratest update better be handled with a separate PR?

brikis98 · 2021-01-25T18:33:58Z

Since I ran into the issue with an incompatible terraform / terratest combination, should I update the referenced terratest version as well and remove the workaround - or should the terratest update better be handled with a separate PR?

Separate PR would be better. Thank you!

MatthiasScholz · 2021-01-31T12:32:58Z

I saw the last test run failed due to "No cluster leader" error reported and the cluster not being able to elect a leader even after some retries. It does not seem to be related to the PR itself. What do you think?

Trying to reproduce the issue running the test again using my AWS account was not successful. It might be a timing issue.

Is there anything I could check further?

brikis98 · 2021-02-01T14:54:52Z

It's hard to tell! There is an intermittent test failure hiding in this repo, as most nightly tests pass, but occasionally, we get a failure. I'm going to re-run the tests to see what happens.

brikis98 · 2021-02-01T15:44:11Z

Yup, looks like an intermittent issue. OK, on another re-ran, tests passed, so it can't be anything related to this PR, as a bug in this PR would probably result in failures consistently. Thanks for the PR/patience. Merging now!

brikis98 · 2021-02-01T15:46:04Z

https://github.com/hashicorp/terraform-aws-nomad/releases/tag/v0.7.2

MatthiasScholz and others added 5 commits December 29, 2020 22:46

✨ making ssh cidr optional

205695a

📝 cleanup dep leftovers

5acca55

✨ making ssh cidr optional

cb4f611

♻️ make cidr blocks configurable

2c960db

✅ ssh access test

846c3ff

Testing SSH access depending on the CIDR block configuration. Nevertheless due to a bug in the terraform CLI: - hashicorp/terraform#17032 test execution is not possible until the bug is solved.

MatthiasScholz requested a review from robmorgan as a code owner January 3, 2021 23:26

brikis98 reviewed Jan 6, 2021

View reviewed changes

examples/nomad-consul-separate-cluster/main.tf Outdated Show resolved Hide resolved

modules/nomad-cluster/main.tf Outdated Show resolved Hide resolved

modules/nomad-cluster/variables.tf Outdated Show resolved Hide resolved

test/README.md Show resolved Hide resolved

MatthiasScholz and others added 2 commits January 7, 2021 19:56

♻️ making count more idiomatic

329b18e

Co-authored-by: Yevgeniy Brikman <brikis98@users.noreply.github.com>

📝 grammar

78a8b3c

Co-authored-by: Yevgeniy Brikman <brikis98@users.noreply.github.com>

brikis98 reviewed Jan 8, 2021

View reviewed changes

examples/nomad-consul-separate-cluster/main.tf Outdated Show resolved Hide resolved

test/nomad_cluster_ssh_test.go Show resolved Hide resolved

brikis98 reviewed Jan 14, 2021

View reviewed changes

🎨 correcting indentation

82027f7

MatthiasScholzTW added a commit to MatthiasScholz/cos that referenced this pull request Jan 17, 2021

💩 using fork to unblock waiting for PR | #85

b30b0df

Waiting for PR to be merge: hashicorp/terraform-aws-nomad#85

MatthiasScholzTW added a commit to MatthiasScholz/cos that referenced this pull request Jan 17, 2021

💩 using fork to unblock waiting for PR | #85

bbf378e

Waiting for PR to be merge: hashicorp/terraform-aws-nomad#85

brikis98 approved these changes Jan 18, 2021

View reviewed changes

🎨 gofmt on all go files

9218915

MatthiasScholz mentioned this pull request Jan 21, 2021

Terraform 0.14.3 breaks Terraform Output Handling gruntwork-io/terratest#766

Closed

🐛 restore gofmt removed import + fixing tests

3f980f3

Add support for terraform >= 0.14.3 using a wrapper function to bypass terratest limitation: - gruntwork-io/terratest#766 Adding missing SSH key pair to make SSH test work.

Merge branch 'master' into master

aaaf615

brikis98 merged commit bf3ee5f into hashicorp:master Feb 1, 2021

MatthiasScholz mentioned this pull request Feb 1, 2021

Making CIDR Block for SSH Configuration Optional and Outbound Traffic Configurable #84

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making CIDR Blocks Configurable #85

Making CIDR Blocks Configurable #85

MatthiasScholz commented Jan 3, 2021 •

edited

Loading

hashicorp-cla commented Jan 3, 2021 •

edited

Loading

brikis98 left a comment

brikis98 left a comment

brikis98 Jan 14, 2021

MatthiasScholz Jan 14, 2021

brikis98 left a comment

brikis98 commented Jan 18, 2021

brikis98 commented Jan 19, 2021

brikis98 commented Jan 19, 2021

brikis98 commented Jan 22, 2021

MatthiasScholz commented Jan 22, 2021

brikis98 commented Jan 25, 2021

MatthiasScholz commented Jan 31, 2021

brikis98 commented Feb 1, 2021

brikis98 commented Feb 1, 2021

brikis98 commented Feb 1, 2021

Making CIDR Blocks Configurable #85

Making CIDR Blocks Configurable #85

Conversation

MatthiasScholz commented Jan 3, 2021 • edited Loading

hashicorp-cla commented Jan 3, 2021 • edited Loading

brikis98 left a comment

Choose a reason for hiding this comment

brikis98 left a comment

Choose a reason for hiding this comment

brikis98 Jan 14, 2021

Choose a reason for hiding this comment

MatthiasScholz Jan 14, 2021

Choose a reason for hiding this comment

brikis98 left a comment

Choose a reason for hiding this comment

brikis98 commented Jan 18, 2021

brikis98 commented Jan 19, 2021

brikis98 commented Jan 19, 2021

brikis98 commented Jan 22, 2021

MatthiasScholz commented Jan 22, 2021

brikis98 commented Jan 25, 2021

MatthiasScholz commented Jan 31, 2021

brikis98 commented Feb 1, 2021

brikis98 commented Feb 1, 2021

brikis98 commented Feb 1, 2021

MatthiasScholz commented Jan 3, 2021 •

edited

Loading

hashicorp-cla commented Jan 3, 2021 •

edited

Loading