search

lablabs / terraform-aws-eks-external-dns

Apache License 2.0
33 stars 26 forks source link

bug: tomap() causes boolean check to fail when var.settings has values for argocd #29

Closed floppyfish14 closed 1 year ago

floppyfish14 commented 1 year ago

Summary

When the settings map has values, and argocd is being used, a boolean check for forceString fails with the following error:

Error: error validating "": error validating data: ValidationError(Application.spec.source.helm.parameters[0].forceString): invalid type for io.argoproj.v1alpha1.Application.spec.source.helm.parameters.forceString: got "string", expected "boolean"

The tomap() function in terraform causes all objects in the function to be converted to the same type. https://developer.hashicorp.com/terraform/language/functions/tomap

In argo.tf you can remove the tomap() function on line 15 and the forceString parameter makes it through as a boolean. This worked on my local machine.

Issue Type

Bug Report

Terraform Version

$ terraform --version
Terraform v1.3.3
on windows_amd64

Your version of Terraform is out of date! The latest version
is 1.3.6. You can update by downloading from https://www.terraform.io/downloads.html

Steps to Reproduce

module "eks-external-dns" {
  source  = "lablabs/eks-external-dns/aws"
  version = "1.1.0"

  cluster_identity_oidc_issuer     = module.eks.oidc_provider
  cluster_identity_oidc_issuer_arn = module.eks.oidc_provider_arn

  # https://registry.terraform.io/modules/lablabs/eks-external-dns/aws/latest#argo-helm
  enabled           = true
  argo_enabled      = true
  argo_helm_enabled = true

  argo_namespace = helm_release.argocd.namespace
  argo_sync_policy = {
    "automated" : {}
    "syncOptions" = ["CreateNamespace=true"]
  }

  settings = {
    "policy" = "sync"
  }
}

Expected Results

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Actual Results

Error: error validating "": error validating data: ValidationError(Application.spec.source.helm.parameters[0].forceString): invalid type for io.argoproj.v1alpha1.Application.spec.source.helm.parameters.forceString: got "string", expected "boolean"
floppyfish14 commented 1 year ago

Created PR: https://github.com/lablabs/terraform-aws-eks-external-dns/pull/30

tomas-balaz commented 1 year ago

Thanks for fixing this bug. Usage of settings input is not so common. The best practice is to put all custom user inputs to the values.

module "eks-external-dns" {
  source  = "lablabs/eks-external-dns/aws"
  version = "1.1.0"

  cluster_identity_oidc_issuer     = module.eks.oidc_provider
  cluster_identity_oidc_issuer_arn = module.eks.oidc_provider_arn

  # https://registry.terraform.io/modules/lablabs/eks-external-dns/aws/latest#argo-helm
  enabled           = true
  argo_enabled      = true
  argo_helm_enabled = true

  values = {
    policy = "sync"
  }
}