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.

Sign up for GitHub

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

Jump to bottom

feat: Upstream bastion component #340

Merged
merged 1 commit into from
Jul 9, 2021
Merged

feat: Upstream bastion component #340

merged 1 commit into from
Jul 9, 2021

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Jul 9, 2021 •
edited
Loading

what

  • Upstream bastion component

why

  • To provide a bastion host

references

notes

To pass pre-commit hook

tfenv install 0.13.5
tfenv use 0.13.5
make readme
pre-commit run --show-diff-on-failure --color=always --all-files
git add modules/bastion

@nitrocode nitrocode requested review from a team as code owners July 9, 2021 15:15
@nitrocode nitrocode force-pushed the bastion branch 3 times, most recently from a59b7dd to c94a363 Compare July 9, 2021 15:42
korenyoni
korenyoni requested changes Jul 9, 2021
View reviewed changes
Copy link
Member

@korenyoni korenyoni left a comment

Choose a reason for hiding this comment

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

Requested minor changes.

modules/bastion/main.tf Outdated Show resolved Hide resolved
modules/bastion/README.md Show resolved Hide resolved
modules/bastion/variables.tf Outdated Show resolved Hide resolved
korenyoni
korenyoni reviewed Jul 9, 2021
View reviewed changes
modules/bastion/main.tf Outdated Show resolved Hide resolved
@nitrocode nitrocode force-pushed the bastion branch 2 times, most recently from 699d02a to 71ec8a7 Compare July 9, 2021 16:01
@nitrocode nitrocode requested a review from korenyoni July 9, 2021 16:03
@nitrocode nitrocode force-pushed the bastion branch 2 times, most recently from 74911a9 to 58770c9 Compare July 9, 2021 16:25
@nitrocode nitrocode force-pushed the bastion branch 3 times, most recently from c7a152a to 5d6d016 Compare July 9, 2021 16:56
@korenyoni korenyoni mentioned this pull request Jul 9, 2021
Merged
korenyoni
korenyoni requested changes Jul 9, 2021
View reviewed changes
Copy link
Member

@korenyoni korenyoni left a comment

Choose a reason for hiding this comment

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

Please bump cloudposse/ec2-bastion-server/aws

modules/bastion/main.tf Outdated Show resolved Hide resolved
@nitrocode nitrocode requested a review from korenyoni July 9, 2021 17:28
@korenyoni korenyoni mentioned this pull request Jul 9, 2021
Merged
korenyoni
korenyoni reviewed Jul 9, 2021
View reviewed changes
modules/bastion/main.tf Outdated Show resolved Hide resolved
korenyoni
korenyoni requested changes Jul 9, 2021
View reviewed changes
Copy link
Member

@korenyoni korenyoni left a comment

Choose a reason for hiding this comment

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

I think it would be a good idea to keep var.ssm_enabled as part of this component instead of abstracting it to the module entirely.

modules/bastion/variables.tf Show resolved Hide resolved
modules/bastion/main.tf Show resolved Hide resolved
@nitrocode nitrocode requested a review from korenyoni July 9, 2021 18:47
@nitrocode nitrocode changed the title Upstream bastion component feat: Upstream bastion component Jul 9, 2021
korenyoni
korenyoni approved these changes Jul 9, 2021
View reviewed changes
Copy link
Member

@korenyoni korenyoni left a comment

Choose a reason for hiding this comment

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

There are still issues with this component, namely the fact that it doesn't support the bastion not associating a public IP if var.associate_public_ip_address=false. But I will address this in a separate PR.

@nitrocode nitrocode merged commit 51398f7 into master Jul 9, 2021
@nitrocode nitrocode deleted the bastion branch July 9, 2021 19:26
cathex-matt pushed a commit to cat-home-experts/terraform-aws-components that referenced this pull request Aug 5, 2021
@nitrocode @cathex-matt
Upstream bastion component (cloudposse#340)
ee3cca5
@renovate renovate bot mentioned this pull request Oct 19, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@korenyoni korenyoni korenyoni approved these changes

@Makeshift Makeshift Awaiting requested review from Makeshift

@RothAndrew RothAndrew Awaiting requested review from RothAndrew

@osterman osterman Awaiting requested review from osterman

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nitrocode @osterman @korenyoni