Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose additional fields in state versions struct #484

Merged
merged 16 commits into from Aug 9, 2022

Conversation

laurenolivia
Copy link
Contributor

@laurenolivia laurenolivia commented Jul 26, 2022

Description

This PR exposes additional attributes in the State Versions API.

Testing plan

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 TestFunctionsAffectedByChange

...

@laurenolivia laurenolivia requested a review from a team as a code owner July 26, 2022 21:57
@laurenolivia laurenolivia changed the title expose additional fields in structs expose additional fields in state versions structs Jul 26, 2022
@laurenolivia laurenolivia changed the title expose additional fields in state versions structs expose additional fields in state versions struct Jul 26, 2022
Comment on lines +64 to +69
ResourcesProcessed bool `jsonapi:"attr,resources-processed"`
StateVersion int `jsonapi:"attr,state-version"`
TerraformVersion string `jsonapi:"attr,terraform-version"`
Modules *StateVersionModules `jsonapi:"attr,modules"`
Providers *StateVersionProviders `jsonapi:"attr,providers"`
Resources []*StateVersionResources `jsonapi:"attr,resources"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandonc I can confirm the new attributes are exposed in the response body. Is that sufficient? Do you have any suggestions?

{
   "data": {
      "id": "sv-DYXpEz7SQBrRJEHV",
      "type": "state-versions",
      "attributes": {
         "created-at": "2022-07-06T17:54:23.254Z",
         "size": 699,
         "hosted-state-download-url": "https://tfcdev-2c13224a.ngrok.io/_archivist/v1/object/dmF1bHQ6djE6MkpXVGFwQmhqWnRheGdGZEpjUVV4UGFCRFhyVm9nTTV3a044SVF3eldHTGtFVHNpSXpYOVRBWHRNNkU0SVlxMVVObjZPajRId0drUGlXN0N4dTdxRmpTeGV6Yk56SmtIUHRCbVJsN3pGNGhpSWswMnNJUWJ4RDhMcEdnMERwZk5ycFRpemhaVWlyVmZXWmZsYkJxYnVQU0tpOGJrOTYvL21NOWswU1ZQRXkxeWF2WTBxTElJK0gzTlBYYmRpbThsUlpxeDZldUVJQ1FYMEN3VHJXS1d4cmVqRzMrT2xHWVl4NktYWlNpTStTWWhzZ2lDYXExMGZLSjN4ekxJVWNYdjVwaDJtM1hCYWlDUEUzNGtaV0luSDZwbTVzZmcxNG5ZQWVQVkJHZzhUWjFBQk1KRFFrd3BDY0dya3ZjRlM4UktITmpKNjlEMG01NXI1QTJtT3dtd3JCMFlwek50bzY5bXkrMnk1V1FPMW9TM2t1c3BITFpocmNMaFNlOTJ4VjVOVmlhSTVKbnVjdHlNTERaU0gxbkRpMWpCTy9XSlNYQT0",
         "modules": {
            "root": {
               "random_pet": 3
            }
         },
         "providers": {
            "provider[\"registry.terraform.io/hashicorp/random\"]": {
               "random_pet": 3
            }
         },
         "resources": [
            {
               "name": "animal_trio",
               "type": "random_pet",
               "count": 3,
               "module": "root",
               "provider": "provider[\"registry.terraform.io/hashicorp/random\"]"
            }
         ],
         "resources-processed": true,
         "serial": 0,
         "state-version": 4,
         "terraform-version": "1.2.4",
         "vcs-commit-url": null,
         "vcs-commit-sha": null
      },
      "relationships": {
         "run": {
            "data": {
               "id": "run-T3EUTgV3yzSR91GE",
               "type": "runs"
            }
         },
         "next-state-version": {
            "data": null
         },
         "previous-state-version": {
            "data": null
         },
         "created-by": {
            "data": {
               "id": "user-wJVRjZegtRxpexoa",
               "type": "users"
            },
            "links": {
               "self": "/api/v2/users/user-wJVRjZegtRxpexoa",
               "related": "/api/v2/runs/run-T3EUTgV3yzSR91GE/created-by"
            }
         },
         "workspace": {
            "data": {
               "id": "ws-LRczoXxb9a3XSmLh",
               "type": "workspaces"
            }
         },
         "outputs": {
            "data": [
               {
                  "id": "wsout-cJjt3xxikeKDGA1g",
                  "type": "state-version-outputs"
               }
            ],
            "links": {
               "related": "/api/v2/state-versions/sv-DYXpEz7SQBrRJEHV/outputs"
            }
         }
      },
      "links": {
         "self": "/api/v2/state-versions/sv-DYXpEz7SQBrRJEHV"
      }
   }
}

@@ -333,6 +333,12 @@ func TestStateVersionsRead(t *testing.T) {
sv.Outputs = nil

assert.Equal(t, svTest, sv)
assert.NotEmpty(t, svTest, svTest.ResourcesProcessed)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I meant! Thanks.

I am slightly worried that some of these will indeed be empty if the backend hasn't yet processed the state. The way to fix this would be to retry while ResourcesProcessed was false. Or we could just wait and see if it flakes!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandonc Wouldn't it stand true that if ResourcesProcessed were false, it would still validate on NotEmpty? Like regardless of which bool, it will still always contain a value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the attr returning false could be an indicator that the other exposed attributes may be empty. Got it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

false is actually empty as well because it's the default value of a boolean. This is the source of a lot of bugs, I think! Check this out:

https://go.dev/play/p/Qsz5Q7FdAWU

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this example in the State Version API docs be incorrect? The response has resources-processed: false but the other attributes I exposed are populated.

"modules": {
          "root": {
              "null-resource": 1,
               "data.terraform-remote-state": 1
                }
            },
            "providers": {
                "provider[\"terraform.io/builtin/terraform\"]": {
                    "data.terraform-remote-state": 1
                },
                "provider[\"registry.terraform.io/hashicorp/null\"]": {
                    "null-resource": 1
                }
            },
            "resources": [
                {
                    "name": "other_username",
                    "type": "data.terraform_remote_state",
                    "count": 1,
                    "module": "root",
                    "provider": "provider[\"terraform.io/builtin/terraform\"]"
                },
                {
                    "name": "random",
                    "type": "null_resource",
                    "count": 1,
                    "module": "root",
                    "provider": "provider[\"registry.terraform.io/hashicorp/null\"]"
                }
            ],
            "resources-processed": false,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is. If resources_processed is false, you would expect the resources key to be empty.

@laurenolivia laurenolivia merged commit aaa4bbb into main Aug 9, 2022
@laurenolivia laurenolivia deleted the laurenolivia/update-structs branch August 9, 2022 18:09
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants