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.

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

ACM submodule: fix bug when not using ssh secret type #679

Merged
merged 8 commits into from
Sep 23, 2020
Merged

ACM submodule: fix bug when not using ssh secret type #679

merged 8 commits into from
Sep 23, 2020

Conversation

cloud-pharaoh
Copy link
Contributor

Module failed as it tried to create git-creds secret regardless of secret type. Add enabled flag to k8sop_creds_secret module.

@comment-bot-dev
Copy link

comment-bot-dev commented Sep 17, 2020

Thanks for the PR! 🚀
✅ Lint checks have passed.

Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

modules/k8s-operator-crd-support/main.tf Outdated Show resolved Hide resolved
@bharathkkb
Copy link
Member

@cloud-pharaoh looks like there maybe some nit formatting issue. terraform fmt --recursive should fix those.

@cloud-pharaoh
Copy link
Contributor Author

Hold off on merging this one. I found an issue with template interpolation.

@cloud-pharaoh
Copy link
Contributor Author

@bharathkkb I am not super happy with creating an ssh key and not using but couldn't find a better solution to the interpolation error. Can you take a look and let me know if there is a better way?

Basically if an existing key is used, local.private_key was null and the k8sop_creds_secret errored out on plan due to an interpolation error.

@bharathkkb
Copy link
Member

@cloud-pharaoh if an existing key is used and var.secret_type == "ssh" shouldn't ssh_auth_key be passed in? That would make local.private_key not null right? Is there another case I am missing?

@cloud-pharaoh
Copy link
Contributor Author

In existing code:

In case of using a different secret type. Not ssh.
secret_type=“gcenode”
create_ssh_key=false
ssh_auth_key=null
local.private_key=null

Module k8sop_cred_secret.enabled will be false and won’t run but even though that’s the case terraform plan runs into an interpolation error because local.private_key is null.

@bharathkkb
Copy link
Member

I see
One kinda hacky way would be to add some logic to the expression itself. This will get rid of the need for unnecessarily creating the ssh key.

kubectl_create_command  = "kubectl create secret generic ${var.operator_credential_name} -n=${var.operator_credential_namespace} --from-literal=${local.k8sop_creds_secret_key}='${var.secret_type != "ssh" && local.private_key == null ? "" : local.private_key}'"

This does seem like a tf limitation. I was able to repro with this minimal config.

locals {
  test_var = null
}

resource "random_pet" "foo" {
  count  = local.test_var == "bar" ? 1 : 0
  prefix = "foo-${local.test_var}"
}

@morgante any thoughts?

@morgante
Copy link
Contributor

What is the underlying error we're encountering here? I don't want to assume that SSH is the only permissable credential type.

It seems like a few fixes are needed:

  1. Update the module to only be activated if var.create_ssh_key = true || var.ssh_auth_key != null
  2. Within the create command, we just need to change the literal to use an inline expression:
kubectl_create_command  = "${local.private_key != null ? "kubectl create secret generic ${var.operator_credential_name} -n=${var.operator_credential_namespace} --from-literal=${local.k8sop_creds_secret_key}='${local.private_key}' : ''}"

@bharathkkb
Copy link
Member

@morgante IIUC the underlying error is we do not want k8sop_creds_secret to be activated if we are not creating a k8s secret for example https://cloud.google.com/anthos-config-management/docs/how-to/installing#using_a_google_source_repository_with_workload_identity. @cloud-pharaoh please correct me if I misunderstood.

@cloud-pharaoh
Copy link
Contributor Author

@bharathkkb correct. The secret was created regardless of secret type. After adding the enabled flag to k8sop_creds_secret the plan command ran into an interpolation error caused by the local.private_key being null. I am testing now with the inline expression as suggested by Morgante to confirm that will resolve it.

@morgante
Copy link
Contributor

@cloud-pharaoh Looking good, just need to reformat.

@cloud-pharaoh
Copy link
Contributor Author

@morgante done :)

Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

LGTM
#688 for the issue we discussed offline

@morgante morgante merged commit 716867c into terraform-google-modules:master Sep 23, 2020
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…terraform-google-modules#679)

* Add enabled flag to git-creds creation

* change to true/false

* fix ssh

* terraform format

* create key regardless to avoid interpolation error

* change enabled condition and add inline expression

* move to module

* format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants