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

Fix update for databricks_storage_credential #1403

Merged
merged 29 commits into from
Jul 7, 2022
Merged

Conversation

nkvuong
Copy link
Contributor

@nkvuong nkvuong commented Jun 27, 2022

This fixes #1402 by making the fields azure_service_principal, aws_iam_role and azure_managed_identity non-array in an update

Add extra tests for update logic, and fix the previous update test for aws_iam_role

@nfx
Copy link
Contributor

nfx commented Jun 27, 2022

@nkvuong please also look for every UC resource, that has an array :)

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2022

Codecov Report

Merging #1403 (d6e2ebd) into master (ad65378) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1403   +/-   ##
=======================================
  Coverage   90.12%   90.13%           
=======================================
  Files         126      126           
  Lines       10187    10194    +7     
=======================================
+ Hits         9181     9188    +7     
  Misses        641      641           
  Partials      365      365           
Impacted Files Coverage Δ
catalog/resource_storage_credential.go 100.00% <100.00%> (ø)
catalog/update.go 100.00% <100.00%> (ø)

@nkvuong
Copy link
Contributor Author

nkvuong commented Jun 27, 2022

no other UC resource has this issue, as they don't have a struct field

@nfx
Copy link
Contributor

nfx commented Jun 28, 2022

@nkvuong could you resolve this small conflict?

wchau and others added 5 commits June 28, 2022 16:03
Bumps [github.com/hashicorp/hcl/v2](https://github.com/hashicorp/hcl) from 2.12.0 to 2.13.0.
- [Release notes](https://github.com/hashicorp/hcl/releases)
- [Changelog](https://github.com/hashicorp/hcl/blob/main/CHANGELOG.md)
- [Commits](hashicorp/hcl@v2.12.0...v2.13.0)

---
updated-dependencies:
- dependency-name: github.com/hashicorp/hcl/v2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/golang-jwt/jwt/v4](https://github.com/golang-jwt/jwt) from 4.4.1 to 4.4.2.
- [Release notes](https://github.com/golang-jwt/jwt/releases)
- [Changelog](https://github.com/golang-jwt/jwt/blob/main/VERSION_HISTORY.md)
- [Commits](golang-jwt/jwt@v4.4.1...v4.4.2)

---
updated-dependencies:
- dependency-name: github.com/golang-jwt/jwt/v4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Serge Smertin <259697+nfx@users.noreply.github.com>
@fludo
Copy link

fludo commented Jul 4, 2022

@nkvuong can you check/fix the failing tests since it blocks the PR ?

alexott and others added 2 commits July 4, 2022 09:46
Otherwise, get following error when compiling:

```
vendor/github.com/hashicorp/hcl/v2/diagnostic_typeparams.go:22:22: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
vendor/github.com/hashicorp/hcl/v2/diagnostic_typeparams.go:22:24: undeclared name: any (requires version go1.18 or later)
```
dependabot bot and others added 13 commits July 7, 2022 10:31
Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.7.4 to 1.7.5.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.7.4...v1.7.5)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Right now, when importing we're reading data, and ignoring the missing resources.  But it
doesn't work well, and users get a cryptic error, like this:

```
>terraform import databricks_cluster.this 12313231232
databricks_cluster.this: Importing from ID "12313231232"...
╷
│ Error: The provider returned a resource missing an identifier during
ImportResourceState. This is generally a bug in the resource implementation for
import. Resource import code should not call d.SetId("") or create an empty
ResourceData. If the resource is missing, instead return an error. Please report this to
the provider developers.
```

With this PR, error message is clear:

```
Error: Cannot import non-existent remote object

While attempting to import an existing object to "databricks_cluster.this",
the provider detected that no object exists with the given id. Only
pre-existing objects can be imported; check that the id is correct and that
it is associated with the provider's configured region or endpoint, or use
"terraform apply" to create a new remote object for this resource.
```
Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.85.0 to 0.86.0.
- [Release notes](https://github.com/googleapis/google-api-go-client/releases)
- [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.85.0...v0.86.0)

---
updated-dependencies:
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.7.5 to 1.8.0.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.7.5...v1.8.0)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Bump Go version for docker & github actions
* Use of go 1.18 instead of 1.16 to support latest hashicorp/hcl/v2

Otherwise, get following error when compiling:

```
vendor/github.com/hashicorp/hcl/v2/diagnostic_typeparams.go:22:22: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
vendor/github.com/hashicorp/hcl/v2/diagnostic_typeparams.go:22:24: undeclared name: any (requires version go1.18 or later)
```
nkvuong and others added 6 commits July 7, 2022 10:34
Bumps [github.com/hashicorp/hcl/v2](https://github.com/hashicorp/hcl) from 2.12.0 to 2.13.0.
- [Release notes](https://github.com/hashicorp/hcl/releases)
- [Changelog](https://github.com/hashicorp/hcl/blob/main/CHANGELOG.md)
- [Commits](hashicorp/hcl@v2.12.0...v2.13.0)

---
updated-dependencies:
- dependency-name: github.com/hashicorp/hcl/v2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Otherwise, get following error when compiling:

```
vendor/github.com/hashicorp/hcl/v2/diagnostic_typeparams.go:22:22: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
vendor/github.com/hashicorp/hcl/v2/diagnostic_typeparams.go:22:24: undeclared name: any (requires version go1.18 or later)
```
Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Approving, but you have to fix this as a follow-up:
#1431

@nfx nfx changed the title Fix update logic for storage credentials Fix update for databricks_storage_credential Jul 7, 2022
@nfx nfx added the bug Something isn't working label Jul 7, 2022
@nfx nfx merged commit 699716a into master Jul 7, 2022
@nfx nfx deleted the feature/fix-credential-update branch July 7, 2022 11:35
@nfx nfx mentioned this pull request Jul 7, 2022
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this pull request Feb 15, 2023
* flatten the array for storage credentials
* add test for updating azmi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
8 participants