microsoft / azure-pipelines-terraform

Azure Pipelines tasks for installing Terraform and running Terraform commands in a build or release pipeline.
MIT License
95 stars 59 forks source link

Change GCP credential filename generation to be deterministic instead of random. #204

Open dmaltsiniotis opened 6 months ago

dmaltsiniotis commented 6 months ago

This PR attempts to address issue #145.

Problem

The behavior of the Terraform Google provider changed after the release of 4.0.0, and this change is slightly incompatible with the way the TerraformCommandHandlerGCP class handles the generation of Google Cloud specific 'credentials' files used for authorizing Terraform to a Google Cloud project (akin to a Service Principle in Azure nomenclature). Specifically, the issue arises during the use of multi-stage pipelines.

Analysis

Historically, when a Terraform plan was saved with the -out parameter, it also embedded a file path to the credential-files.json containing the authorization credentials to the Google project. However, if the credentials-file.json was available via environment variable, the order of precedence would favor environment variable over the embedded path in the plan file. So long as the credential file existed in the "GOOGLE_CREDENTIALS" environment variable, everything would work just fine because this task would generate a new credential file on every stage/job, and set the variable accordingly.

When v4.0.0.0 was published, this order of precedence changed. Now, instead favoring the environment variable "GOOGLE_CREDENTIALS" first, the provider favors the embedded file path first. The reason this becomes an issue in multi-stage pipelines is because of the use of uuid4() to generate the credentials file:

const jsonKeyFilePath = path.resolve(`credentials-${uuidV4()}.json`);

This results in a randomly named file saved to disk. The filename that was used to generate the plan in the first stage, no longer exists on the second/apply stage, since each stage is not guaranteed to be run on the same ephemeral Azure DevOps agent. This will manifest itself is a few possible error message including:

/opt/hostedtoolcache/terraform/1.6.6/x64/terraform apply -auto-approve /home/vsts/work/1/tfplan/main.tfplan
╷
│ Error: the string provided in credentials is neither valid json nor a valid file path
│ 
│ 
╵
##[error]Error: The process '/opt/hostedtoolcache/terraform/1.6.6/x64/terraform' failed with exit code 1
Finishing: Apply Terraform Plan

Solution

Approach

There were a few different proposed solutions to the issue, and I ended up going with the most straightforward change that had the least impact. My goals was to have the task generate a credential filename that was the same for any given build/service connection, so that even between stages if the credential file was re-generated the embedded path in the plan file would remain valid.

My initial thought was to use some kind of MD5 or SHA-1 hashing, and hash a known value to generate the file name. After a quick analysis of the code though, I did not want to introduce any new dependencies as part of this change such as including NodeJS's 'crytpo' or other potentially bloated hashing NPM packages. Luckily, the functionality I was looking for was staring me right in the face: The already included UUID package.

Some quick history on UUID's:

The actual change

This PR updates the jsonKeyFilePath property to instead use uuidV5() instead of uuidV4() This results in a filename that is the deterministic based on the service connection name, and safe for writing to disk on any platform/OS:

const jsonKeyFilePath = path.resolve(`credentials-${uuidV5(serviceName, uuidV5.URL)}.json`);

In this case, the value we are hashing is the serviceName variable. This could just have easily been build ID or anything else that remains constant between stages of a pipeline run that the task has access to.

The namespace (yet another UUID) I chose for uuidV5() is uuidV5.URL, which is a published, known, constant generally used to represent URLs (6ba7b811-9dad-11d1-80b4-00c04fd430c8). There are four preexisting namespace hard-coded GUIDs defined by RFC 4122, in this case it doesn't matter what the namespace is, so long as it is consistent between stages, so I elected to use somewhat friendly looking uuidV5.URL rather than make up some arbitrary UUID value in-line:

const jsonKeyFilePath = path.resolve(`credentials-${uuidV5(serviceName, "12345678-1234-1234-1234-123456789012")}.json`);

Gross, right? But I digress. I made sure to test this all thoroughly with the version of UUID in actual use by the package lock file: 3.4.0.

Testing

This took some time and effort to test. My testing methodology consisted of:

Evidence

First, using the current marketplace version: failing_task_extension

Pipeline runs, fist is marketplace task, second is task from my branch/PR: pipeline_runs

In the current/failing task, notice the UUID is different for the generated credential files between the plan and apply stages. Plan: failing_task_plan

Apply: failing_task_apply

failing_task_apply_error

Now, using the VSTX from this PR: success_task_extension

Using the Terraform task from this PR, we can observe that UUID for the credentials files are now consistent between stages: Plan: success_task_plan

Apply: success_task_apply


Thank you for your consideration.

dmaltsiniotis commented 6 months ago

@dmaltsiniotis please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Demetri Maltsiniotis, LLC"

dmaltsiniotis commented 6 months ago

Bump @mericstam,

Please review at your convenience and let me know if this PR needs additional work or validation.

Thank you,

-Demetri

dmaltsiniotis commented 4 months ago

Not very clear anywhere but we try to follow Azure DevOps sprint number as minor version. (https://whatsprintis.it)

Hi @mericstam, I've updated the version to match the current sprint, please let me know if anything else is needed.

Cheers,

-DM

mericstam commented 4 months ago

Hi, If you don't mind I would like to install the branch changes into out test environment. I you give me a name of a Azure DevOps test-account of your own I can share the published test extension to your account. ( you can create a completely new account if you want). That way we can make sure the extension code works after internal build and deploy. I have very limited GCP knowledge. Your efforts so far are appreciated a lot!

Br Manuel

dmaltsiniotis commented 4 months ago

Hi, If you don't mind I would like to install the branch changes into out test environment. I you give me a name of a Azure DevOps test-account of your own I can share the published test extension to your account. ( you can create a completely new account if you want). That way we can make sure the extension code works after internal build and deploy. I have very limited GCP knowledge. Your efforts so far are appreciated a lot!

Br Manuel

Hi Manuel,

That's no problem at all. I've taken care to test the extension to make sure there are no adverse changes. I have an AzDo instance set up at: https://dev.azure.com/demetri/ I can invite you to the account if you wish for testing, just let me know which e-mail you'd prefer.

Thank you,

Demetri

mericstam commented 3 months ago

Hi, I had some problems with our pipeline project in Azure Devops. That has been sorted out. There is one internal PR in queue to be tested. I will let you know as soon as that other has been tested then I will push your code to the dev feed.

dmaltsiniotis commented 3 months ago

Hi, I had some problems with our pipeline project in Azure Devops. That has been sorted out. There is one internal PR in queue to be tested. I will let you know as soon as that other has been tested then I will push your code to the dev feed.

Wonderful, thank you for the update!

richbjhudson commented 1 month ago

Hi @dmaltsiniotis @mericstam Any update on when this fix will be available would be greatly appreciated.

mericstam commented 1 month ago

Hi @dmaltsiniotis @mericstam Any update on when this fix will be available would be greatly appreciated.

We are having some internal issues with deploying extensions to the marketplace. We are working on that and hopefully it will be fixed ASAP. This PR is first in line after we fixed the pipeline and published the fix for the last deploy