Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

SSH known hosts file management #664

Merged
merged 32 commits into from
Jan 23, 2019
Merged

SSH known hosts file management #664

merged 32 commits into from
Jan 23, 2019

Conversation

JoshVanL
Copy link
Contributor

What this PR does / why we need it:
As per the proposal outlined here #643, this PR implements management of a clusters known hosts file.

During boot, wing will send it's public keys to a Lambda function which is verified and applied as tags to that instance.
Tarmak will then use these tags as a source of truth and will self maintain the known hosts file.
The SSH client is now strictly enforcing the contents of this file when connecting to instances.

Special notes for your reviewer:
The --wing-dev-mode needs to be used during applies with this PR.

Self maintain and strictly enforce the SSH `known_hosts` file.

/hold
Waiting for #494

@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. area/terraform Indicates a PR is affecting Terraform configuration labels Dec 19, 2018
@jetstack-bot jetstack-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 19, 2018
@JoshVanL
Copy link
Contributor Author

/assign

@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 21, 2018
@jetstack-bot jetstack-bot added kind/documentation Categorizes issue or PR as related to documentation. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 2, 2019
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 3, 2019
@JoshVanL
Copy link
Contributor Author

JoshVanL commented Jan 4, 2019

/hold cancel

@jetstack-bot jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2019
@JoshVanL
Copy link
Contributor Author

JoshVanL commented Jan 4, 2019

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL Jan 4, 2019
simonswine
simonswine reviewed Jan 9, 2019
View reviewed changes
Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

Thanks @JoshVanL

My first round of review, still has quite a few rough edges. I am concerned that we will introduce some breaking elements here.

/assign @JoshVanL
/unassign

terraform/amazon/modules/tagging_control/templates/tagging_control_policy.json Show resolved Hide resolved
terraform/amazon/templates/bastion_user_data.yaml.template
ExecStartPre=/bin/sh -c '\
set -e ;\
usermod -a -G ssh_keys wing ;\
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that really necessary to read the public host key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is so wing can read the ssh private keys to sign the document, to prove we have them

terraform/amazon/templates/instance_pools/instance_pool_iam.tf.template Show resolved Hide resolved
terraform/amazon/templates/jenkins_user_data.yaml.template Show resolved Hide resolved
terraform/amazon/templates/jenkins_user_data.yaml.template Outdated Show resolved Hide resolved
terraform/amazon/templates/vault_instances.tf.template Outdated Show resolved Hide resolved
terraform/amazon/templates/vault_instances.tf.template Show resolved Hide resolved
terraform/amazon/modules/bastion/bastion_iam.tf Outdated Show resolved Hide resolved
terraform/amazon/modules/bastion/bastion.tf Show resolved Hide resolved
pkg/tarmak/assets/assets.go
@@ -8,4 +8,5 @@

package assets

//go:generate go-bindata -prefix ../../../ -pkg $GOPACKAGE -o assets_bindata.go ../../../terraform/amazon/modules/... ../../../terraform/amazon/templates/... ../../../puppet/... ../../../packer/...
//go:generate go run ../../../cmd/tagging_control/main.go zip ../../../tagging_control.zip ../../../tagging_control_linux_amd64
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really ugly, not too sure if it break during the release process

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why would this break?

@jetstack-bot jetstack-bot assigned JoshVanL and unassigned simonswine Jan 9, 2019
@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 10, 2019
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2019
@JoshVanL
Copy link
Contributor Author

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL Jan 11, 2019
@JoshVanL
Updates docs and formats terraform code
7f4fe24 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Removes unused Jenkins user data file
2d0cc22 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Adds environment to lambda name with fixes
6991e5c 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Adds in tree zip pkg to zip tagging_control during generate
88a99e2 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Uses cobra for tagging_control function
3efa646 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Remove lambda function ingress SG
24f34a0 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Share wing binary read policy across roles in terraform
d79852c 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Adds a fallback for instances that have no public key tags
f74b18d 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Use iam_policy_role_attachment for shared wing binary read policy
42ad0ff 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Use option ask for strict host checking when in interactive mode
06f23fe 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Disable cmd auto gen tags for tagging_control
97c52c8 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Move important instance tags to awstag_ec2_tag
086fb9c 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Remove quotes from WING_ENVIRONMENT
423a13e 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Removes duplicate variable declarations in terraform
db18013 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2019
@JoshVanL
Updates terraform aws provider to prevent early timeouts on lambda
ac70dff 
function uploads

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Updates depreciated terraform vpc_id
962cd33 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Remove trailing period from private_zone name
66152d1 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Copy link
Contributor Author

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL Jan 17, 2019
@JoshVanL JoshVanL mentioned this pull request Jan 21, 2019
Merged
@simonswine
Copy link
Contributor

I think the release tarmak version might have some issue with this change. And obviously it complicates and slowes down the build process.

Other than that I seem to have a few problems where I need to delete the known_hosts file while terraform-tarmak is failing to connect to the bastion.

I am gonna merge it for now but this will be an area of stabilisation/testing

/approve
/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019
@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: simonswine

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2019
@jetstack-bot jetstack-bot merged commit 3e55dc8 into jetstack:master Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@simonswine simonswine simonswine left review comments

@opeyemi33171 opeyemi33171 Awaiting requested review from opeyemi33171

Assignees

@simonswine simonswine

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/terraform Indicates a PR is affecting Terraform configuration dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JoshVanL @simonswine @jetstack-bot