goharbor / terraform-provider-harbor

A Terraform provider for Harbor. To configure and manage all aspects of your Harbor Container Registry with Terraform Infrastructure as Code.
https://registry.terraform.io/providers/goharbor/harbor
MIT License
118 stars 93 forks source link

Terraform forces replacement if the project-level robot account prefix contains plus sign #479

Open berkitamas opened 3 days ago

berkitamas commented 3 days ago

Describe the bug With the new v3.10.16 the project level robot account breaks if the Robot Name Prefix is set to robot+ or similar (if contains +).

To Reproduce

Outcome: Terraform forces replacement

Expected behavior Terraform reports back "No changes"

Additional context I did a minimal investigation, and the issue is most probably this: https://github.com/goharbor/terraform-provider-harbor/commit/8e20c50506f989771bdb1f919db8ff0ca1ba366f

The line shortName := strings.TrimPrefix(robot.Name, systemConfig.RobotNamePrefix.Value) only exists on the system-level robot account branch while previously it was before the if.

flbla commented 3 days ago

Hi, The line shortName also exist for project : https://github.com/goharbor/terraform-provider-harbor/blob/main/provider/resource_robot_account.go#L162 and this is like this since v3.10.12 : https://github.com/goharbor/terraform-provider-harbor/commit/9d93a41d5349964373f78a40fa6ff2af8a0f7f84

could you please copy your code and logs ?

berkitamas commented 3 days ago

I have to create a bare minimum Terraform code for testing, that takes some time.

In the meantime... That line is taking the 2nd element of the array splitted with "+".

Maybe replacing that line with shortName = robot.Name[strings.LastIndex(robot.Name, "+")+1:] can work but I am far from a professional Go developer so there might be more elegant solutions.

berkitamas commented 3 days ago

In the previous version, the prefix was trimmed before the split: https://github.com/goharbor/terraform-provider-harbor/blob/v3.10.15/provider/resource_robot_account.go#L171

flbla commented 3 days ago

okay I understoud I think we will need extra data from the harbor API

berkitamas commented 3 days ago

Alright, here is a minimal code just in case:

terraform {
  required_providers {
    harbor = {
      source  = "goharbor/harbor"
      version = "= 3.10.16"
    }
  }
}

variable "harbor_url" {
  type = string
}

variable "harbor_username" {
  type = string
}

variable "harbor_password" {
  type      = string
  sensitive = true
}

variable "harbor_project" {
  type = string
}

provider "harbor" {
  url      = var.harbor_url
  username = var.harbor_username
  password = var.harbor_password
}

resource "harbor_robot_account" "robot" {
  level       = "project"
  name        = "robot-pull"
  description = "Robot account for pulling artifacts"
  permissions {
    kind      = "project"
    namespace = var.harbor_project
    access {
      action   = "pull"
      resource = "repository"
    }
  }
}

After the second apply, it starts a force replacement loop:

harbor_robot_account.robot: Refreshing state... [id=/robots/2316]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # harbor_robot_account.robot must be replaced
-/+ resource "harbor_robot_account" "robot" {
      ~ full_name   = "robot+project_name+robot-pull" -> (known after apply)
      ~ id          = "/robots/2316" -> (known after apply)
      ~ name        = "project_name" -> "robot-pull" # forces replacement
      ~ robot_id    = "2316" -> (known after apply)
      ~ secret      = (sensitive value)
        # (4 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 1 to add, 0 to change, 1 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value:
berkitamas commented 3 days ago

Some kind of regular expression might also work: ^.*[a-z0-9]+(?:[._\-][a-z0-9]+)*\+(?P<short_name>[a-z0-9]+(?:[._\-][a-z0-9]+)*)$