digitalocean / terraform-provider-digitalocean

Terraform DigitalOcean provider
https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs
Mozilla Public License 2.0
510 stars 278 forks source link

digitalocean_app: removal changes are not respected #1271

Closed ddelange closed 8 hours ago

ddelange commented 6 days ago

Bug Report


Describe the bug

When removing a part of the digitalocean_app spec, those removals are not recognized as changes:

Affected Resource(s)

Expected Behavior

In each example above, I expect the diff to be propagated and cause a terraform apply with changes.

Actual Behavior

Written for each example, but it looks like the terraform module simply ignores the code changes.

Steps to Reproduce

see diffs above

Terraform Configuration Files see diffs above

Terraform version 1.5.7 (the last open licensed version)

Debug Output N/A

Panic Output N/A

Additional context

Important Factoids

N/A

References

N/A

andrewsomething commented 3 days ago

The core of the problem here is that these attributes are set to both Optional and Computed. This is due to the way App Platform generates defaults for them when not provided and the need to provide backwards compatibility for existing Terraform configurations.

For example, creating an app using a Terraform config like:

resource "digitalocean_app" "golang-sample" {
  spec {
    name   = "golang-sample"
    region = "nyc"

    service {
      name               = "go-service"
      instance_count     = 1
      instance_size_slug = "apps-s-1vcpu-1gb"

      git {
        repo_clone_url = "https://github.com/digitalocean/sample-golang.git"
        branch         = "main"
      }
    }
  }
}

Will create an app with a default ingress and http_port:

{
  "name": "golang-sample",
  "services": [
    {
      "name": "go-service",
      "git": {
        "repo_clone_url": "https://github.com/digitalocean/sample-golang.git",
        "branch": "main"
      },
      "instance_size_slug": "apps-s-1vcpu-1gb",
      "instance_count": 1,
      "http_port": 8080
    }
  ],
  "region": "nyc",
  "ingress": {
    "rules": [
      {
        "match": {
          "path": {
            "prefix": "/"
          }
        },
        "component": {
          "name": "go-service"
        }
      }
    ]
  }
}

Removing Computed from both ingress and http_port would allow for making the service inaccessible from the public internet, but it would effectively make them required. Any new or existing app that does not specify them would be faced with a diff like this on apply:

 # digitalocean_app.golang-sample will be updated in-place
  ~ resource "digitalocean_app" "golang-sample" {
        id                   = "a18c1da0-72de-4203-aef9-360428a1ff12"
        # (8 unchanged attributes hidden)

      ~ spec {
            name     = "golang-sample"
            # (3 unchanged attributes hidden)

          - ingress {
              - rule {
                  - component {
                      - name                 = "go-service" -> null
                      - preserve_path_prefix = false -> null
                        # (1 unchanged attribute hidden)
                    }
                  - match {
                      - path {
                          - prefix = "/" -> null
                        }
                    }
                }
            }

          ~ service {
              - http_port          = 8080 -> null
                name               = "go-service"
                # (8 unchanged attributes hidden)

                # (1 unchanged block hidden)
            }
        }
    }

I believe that if we remove Computed from rule, we should be able to provide a way to maintain backwards compatibility and allow for creating an internal service using a config like:

resource "digitalocean_app" "golang-sample" {
  spec {
    name   = "golang-sample"
    region = "nyc"

    service {
      name               = "go-service"
      instance_count     = 1
      instance_size_slug = "apps-s-1vcpu-1gb"

      git {
        repo_clone_url = "https://github.com/digitalocean/sample-golang.git"
        branch         = "main"
      }

      http_port = 0
      internal_ports = [ 8080 ]
    }

    ingress { }
  }
}

It is slightly unintuitive, but I think this is the best path forward.

ddelange commented 2 days ago

hi @andrewsomething 👋

that sounds good to me! I think both http_port = 0 or http_port = "" are totally fine.