go-tfe icon indicating copy to clipboard operation
go-tfe copied to clipboard

Fix ignored json meta tags for run variables in RunCreateOptions

Open Uk1288 opened this issue 2 years ago • 4 comments

Description:

**NOTE: TheVariables field in Run has been changed from type []*RunVariable to []*RunVariableAttr **

Ref PR: https://github.com/hashicorp/go-tfe/pull/204

Run variables in RunCreateOptions are not marshalled in line with the API (https://www.terraform.io/cloud-docs/api-docs/run#request-body). The variables are currently marshalled as:

"variables": [
        {
          "Key": "aKey",
          "Value": "\"aValue\""
        }
]

instead of

"variables": [
        {
          "key": "aKey",
          "value": "\"aValue\""
        }
]

This PR adds changes that marshals and un-marshals the run variables correctly.

Testing plan

  1. Create a run with options
	newRun, err := client.Runs.Create(context.Background(), tfe.RunCreateOptions{
		Workspace: &tfe.Workspace{
			ID: "ws-ID",
		},
		Variables: []*tfe.RunVariableOption{
			{
				Key:   "name",
				Value: "\"aName\"",
			},
		},
	})
  1. Run should be created with correct variable key, name attributes,
  2. Run value in go client should have Key, Value fields populated as well.

External links

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" TF_ACC="1" go test ./... -v -tags=integration -run TestRunCreateOptions_Marshal

=== RUN   TestRunCreateOptions_Marshal
--- PASS: TestRunCreateOptions_Marshal (1.47s)
PASS
ok 

Uk1288 avatar Sep 13 '22 18:09 Uk1288

@annawinkler It's not a breaking change because the behavior doesn't match the documentation. (It capitalizes the keys in the request, which technically works but it's not expected) So this is just a bug fix.

brandonc avatar Sep 13 '22 20:09 brandonc

@annawinkler I see now that you mean modifying the actual type. Let me think about it. It would probably be less impactful to add a new output type and redefine the existing input type.

brandonc avatar Sep 13 '22 20:09 brandonc

Hmm looks like StateVersion is flaking on your PR 🤔 But I checked out this branch and you have the latest changes.

laurenolivia avatar Sep 15 '22 21:09 laurenolivia

Testing plan works as expected! ✅

laurenolivia avatar Sep 16 '22 14:09 laurenolivia

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

github-actions[bot] avatar Sep 27 '22 20:09 github-actions[bot]