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

Terraform Plugin SDK Upgrade #379

Merged
merged 24 commits into from
Mar 15, 2021
Merged

Terraform Plugin SDK Upgrade #379

merged 24 commits into from
Mar 15, 2021

Conversation

trentrosenbaum
Copy link
Contributor

@trentrosenbaum trentrosenbaum commented Mar 10, 2021

The focus of this PR is to update the Terraform Plugin SDK dependency from v1 to v2. The specific version at the time of this PR is v2.4.4.

  • Update resource and data source functions to be context aware, (removal of deprecated CRUD functions).
  • Switched to use wider context and diag aware functions
    • SchemaValidateDiagFunc
    • StateContext
    • WaitForStateContext
    • ConfigureContextFunc
  • Pruning of logging attributes not present in schema
  • Adds new diag warning when TLS replace attribute returns true
  • Service read function checks for context cancellation
  • Introduction of tfproviderlint and adjustment to address highest warnings

bengesoff added 18 commits March 8, 2021 09:49
Initial upgrade to get everything compiling with the new version.
Doesn't tackle any deprecations and doesn't have a working test suite yet.

Changes are mostly a direct find/replace of the SDK imports, with some more significant changes:
- The UserAgent generation was changed to account for an SDK change (fastly/config.go)
- The hashcode package was removed from the SDK and it is needed in some of the data source code, so it has been copied back in locally (fastly/hashcode/hashcode.go)
- Removal of d.SetPartial and d.Partial as it's been deprecated and doesn't seem to do anything
- Addition of context to CustomizeDiffFunc as the signature has been updated in-place
- A couple of lint errors that came up while trying to commit
A lot of the logging block tests for the compute service were panicking
with the new SDK due to d.Set errors. These errors were ignored in the
resource Read functions, which would previously not have caused an issue
but now result in panics. The error was caused by trying to set VCL
logging attributes on compute services when these fields aren't present
in the schema.

This commit avoids the error by pruning these fields on Compute services
prior to calling d.Set.
Testing TypeSet elements has changed in acceptance tests. Looking up a
preknown hash in the set no longer works, but there are new
TestCheckTypeSetElemAttr and TestCheckTypeSetElemNestedAttrs functions
for this instead which use "set.*" as the syntax.

Also the test was working incorrectly with a "count" attribute. I'm
guessing this worked before because a fake terraform implementation was
being used, but I couldn't get this working now. I tried ....content[0]
to try indexing the first instance of the "count"ed resource but it
still didn't work. In the end, I noticed that it was a bit redundant
anyway because there was always one thing being created, and the rest of
the fields would all have been duplicated if there were multiple.
Subscription Validation resource not compiling as I need to change the retry mechanism
Used twice, once in the Service State Importer function, and once in the Subscription Validation Retry function. These are both context-aware but don't use diag.Diagnostics so need some conversion in order to work with the context-aware resource.*Read functions.
Quite a bit of coercion to get everyone happy because the signature has changed quite a bit from:

  func(interface{}, string) ([]string, []error)

 to:

   func(interface{}, cty.Path) diag.Diagnostics

Which requires a diagsToWarnsAndErrs function and cty.GetAttrPath() in the unit tests for all of the validators.
The validators themselves use the validation.ToDiagFunc() function to do this conversion which is quite simple, and also required for all of the builtin validation helper methods which have weirdly not yet been updated to SchemaValidateDiagFuncs.
…and remove logic for pre-v0.12.0

There was some logic for constructing the User Agent in TF versions earlier than v0.12.0 but those versions are now deprecated and unsupported by the plugin SDK anyway.

Had to use the fmt string for the sweeper User Agent but it should be the same as it was before.
Doesn't fix all the errors but gets a decent bunch of them.
@Integralist Integralist added the enhancement New feature or request label Mar 11, 2021
@trentrosenbaum trentrosenbaum marked this pull request as ready for review March 11, 2021 10:38
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Thanks @trentrosenbaum this LGTM.

I'll approve although I have a couple of small queries.

@Integralist Integralist merged commit 87c24f9 into fastly:master Mar 15, 2021
@trentrosenbaum trentrosenbaum deleted the sdk-upgrade-squashed branch March 15, 2021 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants