Skip to content
This repository has been archived by the owner on Dec 16, 2020. It is now read-only.

Pure Terraform TLS Management #26

Merged
merged 21 commits into from
May 16, 2019
Merged

Pure Terraform TLS Management #26

merged 21 commits into from
May 16, 2019

Conversation

yorinasub17
Copy link
Contributor

@yorinasub17 yorinasub17 commented May 1, 2019

This implements modules to do TLS management in pure terraform using the kubernetes and tls providers, as an alternative to using kubergrunt. In this model, the user does not need to install kubergrunt at all to use all the features. As a part of this, this updates the root example to use the pure terraform version, while migrating the kubergrunt version to the examples folder.

Note that the major drawback of this approach is that the TLS cert information will leak into the Terraform state file.

This is understandably a big PR and a big change. We do NOT have to merge this right away with launch so close.

@yorinasub17
Copy link
Contributor Author

@rileykarson I have rebased the changes and the diff is now correct.

@yorinasub17
Copy link
Contributor Author

NOTES ON EXAMPLE CHANGES:

  • The root example is now the version that uses the TLS modules.
  • There is a new example, k8s-tiller-kubergrunt-minikube, which is the old version of the root example with kubergrunt. There are NO CHANGES to that other than b2c40a8, so you can skip the review if you would like.

description = "The name of the service account to use for Tiller."
}

variable "tls_subject" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we provide defaults like we do in other modules now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

@yorinasub17
Copy link
Contributor Author

UPDATE: Rebased on master to resolve merge conflicts

Copy link
Contributor

@autero1 autero1 left a comment

Choose a reason for hiding this comment

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

This looks good! Sorry for the delay in reviewing... The review session was (again) very helpful.

// os.Setenv("SKIP_create_terratest_options", "true")
// os.Setenv("SKIP_terraform_apply", "true")
// os.Setenv("SKIP_validate", "true")
// os.Setenv("SKIP_cleanup", "true")
Copy link
Contributor

Choose a reason for hiding this comment

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

add // os.Setenv("SKIP_validate_upgrade", "true")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

})

test_structure.RunTestStage(t, "validate_upgrade", func() {
// Make sure the upgrade command mentioned in the docs actually works
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@robmorgan robmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

@yorinasub17
Copy link
Contributor Author

Thanks for the review folks! I'm going to merge and release this now.

@yorinasub17 yorinasub17 merged commit 1a06c66 into master May 16, 2019
@yorinasub17 yorinasub17 deleted the yori-tf-tls-mgmt branch May 16, 2019 04:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants