taiidani / terraform-provider-jenkins

Jenkins Terraform Provider
https://registry.terraform.io/providers/taiidani/jenkins/latest
Apache License 2.0
76 stars 42 forks source link

XML quotation mark entities not handled properly resulting in loss of idempotency #132

Closed mike-sol closed 2 months ago

mike-sol commented 1 year ago

When Jenkins runs, it modifies the job XML to convert stored single and double quotation marks in e.g. inline Pipeline scripts to use XML entities.

This results in diffs like this:

      post {
                    always {
                      script {
                        slackNotify(
          -               type: 'deploy',
          +               type: 'deploy',
                          build: currentBuild,
          -               channel: '#it-notifications',
          -               environment: 'it',
          -               version: '',
          -               component: "it-nomad/job-sapi-documentation-internal"
          +               channel: '#it-notifications',
          +               environment: 'it',
          +               version: '',
          +               component: "it-nomad/job-sapi-documentation-internal"
                        )
                        tf.standardCleanup(itUniqueKey)
                      }
                    }
                  }

Ideally, I think this escaping would probably happen at the provider level when preparing the loaded XML definition so that running a Jenkins job doesn't result in it differing from the Terraform codebase.

Happy to provide more detail if needed.

abrockmeyer-govtact commented 10 months ago

I handled this by following this stack overflow: StackOverflow

resource "local_file" "this2" {
  for_each = local.test_jobs
  content = join("\n", [for line in split("\n", data.jenkins_job.this2["existing_job"].template) :
    format(
      replace(line, "/${join("|", keys(local.map))}/", "%s"),
      [
        for value in flatten(regexall("${join("|", keys(local.map))}", line)) :
        lookup(local.map, value)
      ]...
    )
  ])
  filename = "./test_configs/${each.value}"
}

with local.map

map = {
    ">"   = ">",
    "&lt;"   = "<",
    "&amp;"  = "&",
    "&quot;" = "\"",
    "&apos;" = "'",
    "&nbsp;" = " "
  }
mike-sol commented 9 months ago

That is one hell of a workaround. It's a bit much for me because I'm using the provider with some heavy templating and this would be pretty awkward to add, but if @taiidani isn't able to help with a fix upstream I may do that.

@taiidani let me know if a little bribery would help you find some time here...

taiidani commented 9 months ago

@mike-sol I'll admit I'm having trouble making the time to implement a fix, as the provider is in the midst of a couple migrations (SDKv2 to Plugin Framework, DIY integration tests to Terraform Test) that are taking a lot of that time.

It's on my radar though, and I always appreciate PRs!

taiidani commented 9 months ago

@mike-sol Do you think you could provide a working reproduction? I stole some time this morning trying to come up with something consistent but couldn't reproduce. The provider will perform an unescape on both sides (local/remote) of the plan before generating a diff, so the differences you identified kept getting washed out.

I added an inline pipeline script here for testing against which uses XML entities, versus the existing freestyle job which uses unescaped characters.

github-actions[bot] commented 3 months ago

Stale issue message