Use filepath on user provided ingress#2185
Use filepath on user provided ingress#2185alexouzounis wants to merge 5 commits intocoreos:masterfrom
Conversation
|
Can one of the admins verify this patch? |
|
I tried to do something smart with count or interpolation to have one module but the source filed cannot be interpolated in TF. If anyone has any other ideas, please let me know. The solution I see is to merge the |
squat
left a comment
squat
left a comment
There was a problem hiding this comment.
@alexouzounis thanks for this contribution. Overall this looks ok. Please make the requested changes and then we can run the tests.
platforms/aws/variables.tf
Outdated
| default = "" | ||
|
|
||
| description = <<EOF | ||
| (optional) File with contents the public CA certificate in PEM format |
There was a problem hiding this comment.
These comments are a little confusing. They say to provide a path but then show the contents of the file. Please make these comments match the descriptions for the other PEM path variables, e.g. https://github.com/coreos/tectonic-installer/blob/master/config.tf#L202
platforms/aws/variables.tf
Outdated
| EOF | ||
| } | ||
|
|
||
| variable "tectonic_ingress_ca_cert_pem_path" { |
There was a problem hiding this comment.
These variables are not specific to AWS. Please move them out of the AWS-specific variables.tf and into config.tf.
| ca_key_pem = "${module.kube_certs.ca_key_pem}" | ||
| } | ||
|
|
||
| //module "ingress_certs" { |
There was a problem hiding this comment.
Since this is not AWS-specific, please add this into the Azure, Openstack, bare-metal, and GCP Terraform as well.
|
Sure, yeah. Thanks for having a look so quickly. Will come back to you end of next week. |
|
Hey @squat - let me know what you think about the latest changes when you get some time :) |
|
We did some changes (#2082) to the testing process. Please rebase on to current master, so that the |
|
hey @alexouzounis thanks a lot for this contribution. Part of the reasoning behind embedding was ease of content sharing within the tf space. Let's close this for now as we are currently under considerable refactor of master and the way we generate/consume certificates might be impacted as well |
5 participants
Instead of having to append the contents of the certificates, use file paths
A separate PR would be required to updates the docs; https://github.com/coreos/tectonic-docs/blob/master/Documentation/reference/tls-certificates.md