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

Permanent diff when using consul_config_entry for intentions #281

Closed
lawliet89 opened this issue Aug 19, 2021 · 7 comments · Fixed by #288
Closed

Permanent diff when using consul_config_entry for intentions #281

lawliet89 opened this issue Aug 19, 2021 · 7 comments · Fixed by #288

Comments

@lawliet89
Copy link
Contributor

Hi there,

Thank you for opening an issue. Please note that we try to keep the Terraform issue tracker reserved for bug reports and feature requests. For general usage questions, please see: https://www.terraform.io/community.html.

Terraform Version

Run terraform -v to show the version. If you are not running the latest version of Terraform, please upgrade because your issue may have already been fixed.

Terraform v1.0.5
on linux_amd64
+ provider registry.terraform.io/hashicorp/consul v2.12.0

Affected Resource(s)

Please list the resources as a list, for example:

  • consul_config_entry

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "consul_config_entry" "service_intention" {
  kind = "service-intentions"
  name = "grafana"

  config_json = jsonencode(
    {
      Sources = concat(
        [
          {
            Name        = "ambassador"
            Description = "Allow Ambassador to access"
            Permissions = [
              {
                Action = "allow"
                HTTP = {
                  PathPrefix = "/"
                }
              }
            ]
          }
        ],
        [for service in var.connect_allowed_services : {
          Name        = service
          Description = "Allow service to access"
          Permissions = [
            {
              Action = "allow"
              HTTP = {
                PathPrefix = "/"
              }
            }
          ]

        }],
        [
          {
            Name        = "*"
            Description = "Deny everyone else"
            Permissions = [
              {
                Action = "deny"
                HTTP = {
                  PathPrefix = "/"
                }
              }
            ]
          }
        ],
      )
    }
  )
}

Debug Output

Please provider a link to a GitHub Gist containing the complete debug output: https://www.terraform.io/docs/internals/debugging.html. Please do NOT paste the debug output in the issue; just paste a link to the Gist.

Panic Output

If Terraform produced a panic, please provide a link to a GitHub Gist containing the output of the crash.log.

Expected Behavior

No permanent diff

Actual Behavior

Permanent diff in some values that are assigned by Consul.

Terraform will perform the following actions:

  # consul_config_entry.service_intention will be updated in-place
  ~ resource "consul_config_entry" "service_intention" {
      ~ config_json = jsonencode(
          ~ {
              ~ Sources = [
                  ~ {
                      - Precedence  = 9 -> null
                      - Type        = "consul" -> null
                        # (3 unchanged elements hidden)
                    },
                  ~ {
                      - Precedence  = 8 -> null
                      - Type        = "consul" -> null
                        # (3 unchanged elements hidden)
                    },
                ]
            }
        )
        id          = "service-intentions-grafana"
        name        = "grafana"
        # (1 unchanged attribute hidden)
    }

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform plan
@remilapeyre
Copy link
Collaborator

Hi @lawliet89, thanks for reporting this issue. Could you please add the content of the connect_allowed_services variable so that I can reproduce your configuration locally?

@lawliet89
Copy link
Contributor Author

@remilapeyre var.connect_allowed_services is just an arbitrary list of strings. You can just test it out with [] and you should still be able to reproduce this.

I am using Consul 1.10.1.

@remilapeyre
Copy link
Collaborator

Hi @lawliet89, here's the error I get when trying to apply

locals {
  connect_allowed_services = ["hello", "world"]
}

resource "consul_config_entry" "service_intention" {
  kind = "service-intentions"
  name = "grafana"

  config_json = jsonencode(
    {
      Sources = concat(
        [
          {
            Name        = "ambassador"
            Description = "Allow Ambassador to access"
            Permissions = [
              {
                Action = "allow"
                HTTP = {
                  PathPrefix = "/"
                }
              }
            ]
          }
        ],
        [for service in local.connect_allowed_services : {
          Name        = service
          Description = "Allow service to access"
          Permissions = [
            {
              Action = "allow"
              HTTP = {
                PathPrefix = "/"
              }
            }
          ]

        }],
        [
          {
            Name        = "*"
            Description = "Deny everyone else"
            Permissions = [
              {
                Action = "deny"
                HTTP = {
                  PathPrefix = "/"
                }
              }
            ]
          }
        ],
      )
    }
  )
}

:

│ Error: Failed to set 'grafana' config entry: Unexpected response code: 500 (service "grafana" has protocol "tcp", which is incompatible with L7 intentions permissions)
│ 
│   with consul_config_entry.service_intention,
│   on example.tf line 5, in resource "consul_config_entry" "service_intention":
│    5: resource "consul_config_entry" "service_intention" {

@lawliet89
Copy link
Contributor Author

You need to set a service-defaults to set grafana as a HTTP service.

resource "consul_config_entry" "service_default" {
  kind = "service-defaults"
  name = "grafana"

  config_json = jsonencode({
    Protocol = "http"
  })
}

@remilapeyre
Copy link
Collaborator

Thanks, I looked into the issue. The root cause of the issue is that Consul configuration entries can take different attributes based on their kind, and those attributes can have different defaults based on the configuration of thee cluster. Even worse one config entry kind can have different attributes based on the Consul server version.

This makes it impossible for Terraform to known if an attribute it reads but that was not in the configuration is just the default value and should be kept, or if the config entry was changed by an external process and this attribute needs to be removed. The only way to not have perpetual diffs with config_entry is to define all attributes explicitly:

resource "consul_config_entry" "service_default" {
  kind = "service-defaults"
  name = "grafana"

  config_json = jsonencode({
    Protocol         = "http"
    Expose           = {}
    MeshGateway      = {}
    TransparentProxy = {}
  })
}

locals {
  connect_allowed_services = ["hello", "world"]
}

resource "consul_config_entry" "service_intention" {
  kind = "service-intentions"
  name = "grafana"

  config_json = jsonencode(
    {
      Sources = concat(
        [
          {
            Name        = "ambassador"
            Description = "Allow Ambassador to access"
            Precedence  = 9
            Type        = "consul"
            Permissions = [
              {
                Action = "allow"
                HTTP = {
                  PathPrefix = "/"
                }
              }
            ]
          }
        ],
        [for service in local.connect_allowed_services : {
          Name        = service
          Description = "Allow service to access"
          Precedence  = 9
          Type        = "consul"
          Permissions = [
            {
              Action = "allow"
              HTTP = {
                PathPrefix = "/"
              }
            }
          ]

        }],
        [
          {
            Name        = "*"
            Description = "Deny everyone else"
            Precedence  = 8
            Type        = "consul"
            Permissions = [
              {
                Action = "deny"
                HTTP = {
                  PathPrefix = "/"
                }
              }
            ]
          }
        ],
      )
    }
  )

  depends_on = [
    consul_config_entry.service_default
  ]
}

I see that there is no mention of this in the documentation, since it's something that differs from all other Terraform resources I will add a note about it at https://registry.terraform.io/providers/hashicorp/consul/latest/docs/resources/config_entry.

Do this solves your issue?

@lawliet89
Copy link
Contributor Author

lawliet89 commented Aug 24, 2021

That's logical. The only issue I can see is that the precedence of an intention source might change between different consul versions (AFAIK) . I guess it might not be very viable to put in custom diff suppression code for different config entries?

@remilapeyre
Copy link
Collaborator

Indeed the custom diff suppression function would need to be different for different Consul versions. To make things more complex, Terraform providers needs to be able to be able to compute plans without making calls to the Consul server (e.g. with terraform plan -refresh=false) so we can't determine the version of Consul at this time to know what the defaults should be. I think I will have more possibilities to tackle this once we upgrade the Terraform SDK to the latest version.

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