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

[Feature] Add support partitions in policy data sources #4181

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

ashenm
Copy link
Contributor

@ashenm ashenm commented Oct 31, 2024

Changes

Tests

  • make test run locally
  • relevant change in docs/ folder
  • covered with integration tests in internal/acceptance
  • relevant acceptance tests are passing
  • using Go SDK

@ashenm ashenm requested review from a team as code owners October 31, 2024 15:49
@ashenm ashenm requested review from hectorcast-db and removed request for a team October 31, 2024 15:49
@ashenm ashenm changed the title Add support partitions in policy data sources [Feature] Add support partitions in policy data sources Oct 31, 2024
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

overall looks good, thank you - just small nits.

@nkvuong wdyt?

aws/data_aws_bucket_policy.go Outdated Show resolved Hide resolved
aws/data_aws_unity_catalog_policy.go Outdated Show resolved Hide resolved
docs/data-sources/aws_bucket_policy.md Outdated Show resolved Hide resolved
@ashenm ashenm force-pushed the data-sources/aws-policies branch from c7ed0d0 to c7febf8 Compare October 31, 2024 19:45
@ashenm ashenm requested a review from alexott October 31, 2024 19:49
@ashenm ashenm force-pushed the data-sources/aws-policies branch 3 times, most recently from cb5257c to a427268 Compare November 1, 2024 10:18
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

looks mostly good, just some comments

aws/data_aws_unity_catalog_assume_role_policy.go Outdated Show resolved Hide resolved
common/aws.go Outdated Show resolved Hide resolved
aws/data_aws_unity_catalog_policy.go Show resolved Hide resolved
@ashenm ashenm temporarily deployed to test-trigger-is November 3, 2024 12:13 — with GitHub Actions Inactive
@ashenm ashenm force-pushed the data-sources/aws-policies branch from a427268 to 0c45e23 Compare November 3, 2024 15:12
@ashenm ashenm temporarily deployed to test-trigger-is November 3, 2024 16:02 — with GitHub Actions Inactive
@ashenm
Copy link
Contributor Author

ashenm commented Nov 3, 2024

@hectorcast-db please take a stab at this

Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

@ashenm I did look a bit deeper, and here are additional comments that we need to address:

@ashenm ashenm force-pushed the data-sources/aws-policies branch 3 times, most recently from c0d4d18 to dc244d4 Compare November 6, 2024 11:11
@ashenm
Copy link
Contributor Author

ashenm commented Nov 6, 2024

@alexott

https://github.com/databricks/terraform-provider-databricks/blob/main/aws/data_aws_assume_role_policy.go isn't modified, especially, we need to double-check this one: https://github.com/databricks/terraform-provider-databricks/blob/main/aws/data_aws_assume_role_policy.go#L54 - the log delivery role is hardcoded

Yah nice cath! Updated all log delivery, unity catalogue, account id references https://github.com/databricks/terraform-provider-databricks/compare/5ec45228a6784ff4b1e1402b6d6ab4d2938a8a84..dc244d4375e309f9d14963be4785619c90ff0df1

for aws bucket policy, first, databricks_account_id isn't documented at all, and we need to put a comment there saying that it should be changed for non-aws partition

Yep not just bucket policy assume role policy too don't have databricks_account_id listed as a parameter; added depreciation message on both and to resolev account id depending on partition being selected when not provided

@ashenm ashenm requested a review from alexott November 6, 2024 11:18
@ashenm ashenm temporarily deployed to test-trigger-is November 6, 2024 11:19 — with GitHub Actions Inactive
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

lgtm

@alexott alexott requested a review from nkvuong November 6, 2024 11:25
@ashenm
Copy link
Contributor Author

ashenm commented Nov 7, 2024

@alexott any other codeowners apart from @nkvuong?

@ashenm ashenm force-pushed the data-sources/aws-policies branch from dc244d4 to 66d3d10 Compare November 8, 2024 05:08
@ashenm
Copy link
Contributor Author

ashenm commented Nov 8, 2024

Thanks @nkvuong but looks like yours and @alexott's approvals aint enough 😬

@ashenm ashenm temporarily deployed to test-trigger-is November 8, 2024 07:32 — with GitHub Actions Inactive
@ashenm
Copy link
Contributor Author

ashenm commented Nov 8, 2024

Not sure about the branch protection rules maybe @mgyucht @tammyma-db @nfx @hectorcast-db maybe one of y'all can additionally approve? (judging by past commits on these files :P)

@alexott alexott enabled auto-merge November 8, 2024 08:38
@ashenm
Copy link
Contributor Author

ashenm commented Nov 8, 2024

Thanks @alexott one more help how to trigger the integration tests? It seems it's awaiting its succession

@alexott
Copy link
Contributor

alexott commented Nov 8, 2024

it will be merged when tests finished. don't worry...

@ashenm
Copy link
Contributor Author

ashenm commented Nov 8, 2024

auto-merge was automatically disabled November 8, 2024 17:26

Head branch was pushed to by a user without write access

@ashenm ashenm force-pushed the data-sources/aws-policies branch from 66d3d10 to 96a4029 Compare November 8, 2024 17:26
Copy link

github-actions bot commented Nov 8, 2024

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4181
  • Commit SHA: 96a4029b27fe3fe6f7a6db03a9021025c360dbf8

Checks will be approved automatically on success.

@ashenm ashenm temporarily deployed to test-trigger-is November 10, 2024 11:59 — with GitHub Actions Inactive
@ashenm
Copy link
Contributor Author

ashenm commented Nov 10, 2024

@alexott thanks for triggering those worflows; https://go/deco-tests-run/terraform must be also triggered manually I beilieve since my user don't have access to org secrets like DECO_WORKFLOW_TRIGGER_APP_ID?

(Cus once again test triggers are skipped https://github.com/databricks/terraform-provider-databricks/actions/runs/11746695258/job/32770584952?pr=4181 :/)

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/11777381229

@ashenm
Copy link
Contributor Author

ashenm commented Nov 11, 2024

@alexott any chance to see what's failing on the integration tests?

@ashenm
Copy link
Contributor Author

ashenm commented Nov 11, 2024

Nvmd; bucket policy integrations have a overriding test bucket (in place of standard databricks aws account) fill allow override

@alexott
Copy link
Contributor

alexott commented Nov 11, 2024

it's not a relevant test, we just need to fix the environment. don't worry - PR will be merged when environment is repaired

@alexott alexott enabled auto-merge November 12, 2024 13:11
@alexott alexott added this pull request to the merge queue Nov 12, 2024
Merged via the queue into databricks:main with commit d4e461c Nov 12, 2024
12 checks passed
@ashenm ashenm deleted the data-sources/aws-policies branch November 12, 2024 13:35
hectorcast-db added a commit that referenced this pull request Nov 20, 2024
### New Features and Improvements

 * Add `databricks_mws_network_connectivity_config` and `databricks_mws_network_connectivity_configs` data source ([#3665](#3665)).
 * Add support partitions in policy data sources ([#4181](#4181)).
 * Added `databricks_registered_model_versions` data source ([#4100](#4100)).
 * Update databricks_permissions resource to support vector-search-endpoints ([#4209](#4209)).
 * add `databricks_serving_endpoints` data source ([#4226](#4226)).

### Bug Fixes

 * Add validation for `run_as_mode` in `databricks_query` ([#4233](#4233)).
 * Correct handling of updates for empty comments and `force_destroy` in UC catalog, schema, registered models and volumes ([#4244](#4244)).
 * Fix deletion of dashboard if it was trashed out of band ([#4235](#4235)).
 * Fix waiting for `databricks_vector_search_index` readiness ([#4243](#4243)).
 * Remove single-node validation from interactive clusters ([#4222](#4222)).
 * Remove single-node validation from jobs clusters ([#4216](#4216)).
 * Use cluster list API to determine pinned cluster status ([#4203](#4203)).
 * fix issue cased by setting pause_status in update monitor  ([#4242](#4242)).

### Documentation

 * Clarify workspace provider config ([#4208](#4208)).
 * Update "Databricks Workspace Creator" permissions on gcp-workspace.md ([#4201](#4201)).
 * Update `grants.md` references ([#4246](#4246)).
 * Update description of `group_id` in `databricks_mws_ncc_private_endpoint_rule` ([#4238](#4238)).
 * remove subnet sharing limitation in AWS ([#4239](#4239)).

### Internal Changes

 * Bump Go SDK to latest and generate TF structs ([#4249](#4249)).
 * Mark TestUcAccModelServingProvisionedThroughput as flaky. to be rever… ([#4232](#4232)).
 * Rename resources directory to products in pluginframework ([#4139](#4139)).
 * Revert "mark TestUcAccModelServingProvisionedThroughput as flaky. to … ([#4240](#4240)).
 * Set user agent in some resources implemented in plugin framework ([#4187](#4187)).
 * make `ApplyAndExpectData` work with nested set ([#4237](#4237)).

### Dependency Updates

 * Bump dependencies for Plugin Framework and SDK v2 ([#4215](#4215)).
 * Bump github.com/hashicorp/hcl/v2 from 2.22.0 to 2.23.0 ([#4236](#4236)).
 * Bump github.com/hashicorp/terraform-plugin-testing from 1.10.0 to 1.11.0 ([#4247](#4247)).

### Exporter

 * Add `List` operation for `users` service ([#4204](#4204)).
 * Fix interactive selection of services ([#4245](#4245)).
hectorcast-db added a commit that referenced this pull request Nov 20, 2024
 * Add `databricks_mws_network_connectivity_config` and `databricks_mws_network_connectivity_configs` data source ([#3665](#3665)).
 * Add support partitions in policy data sources ([#4181](#4181)).
 * Added `databricks_registered_model_versions` data source ([#4100](#4100)).
 * Update databricks_permissions resource to support vector-search-endpoints ([#4209](#4209)).
 * add `databricks_serving_endpoints` data source ([#4226](#4226)).

 * Add validation for `run_as_mode` in `databricks_query` ([#4233](#4233)).
 * Correct handling of updates for empty comments and `force_destroy` in UC catalog, schema, registered models and volumes ([#4244](#4244)).
 * Fix deletion of dashboard if it was trashed out of band ([#4235](#4235)).
 * Fix waiting for `databricks_vector_search_index` readiness ([#4243](#4243)).
 * Remove single-node validation from interactive clusters ([#4222](#4222)).
 * Remove single-node validation from jobs clusters ([#4216](#4216)).
 * Use cluster list API to determine pinned cluster status ([#4203](#4203)).
 * fix issue cased by setting pause_status in update monitor  ([#4242](#4242)).

 * Clarify workspace provider config ([#4208](#4208)).
 * Update "Databricks Workspace Creator" permissions on gcp-workspace.md ([#4201](#4201)).
 * Update `grants.md` references ([#4246](#4246)).
 * Update description of `group_id` in `databricks_mws_ncc_private_endpoint_rule` ([#4238](#4238)).
 * remove subnet sharing limitation in AWS ([#4239](#4239)).

 * Bump Go SDK to latest and generate TF structs ([#4249](#4249)).
 * Mark TestUcAccModelServingProvisionedThroughput as flaky. to be rever… ([#4232](#4232)).
 * Rename resources directory to products in pluginframework ([#4139](#4139)).
 * Revert "mark TestUcAccModelServingProvisionedThroughput as flaky. to … ([#4240](#4240)).
 * Set user agent in some resources implemented in plugin framework ([#4187](#4187)).
 * make `ApplyAndExpectData` work with nested set ([#4237](#4237)).

 * Bump dependencies for Plugin Framework and SDK v2 ([#4215](#4215)).
 * Bump github.com/hashicorp/hcl/v2 from 2.22.0 to 2.23.0 ([#4236](#4236)).
 * Bump github.com/hashicorp/terraform-plugin-testing from 1.10.0 to 1.11.0 ([#4247](#4247)).

 * Add `List` operation for `users` service ([#4204](#4204)).
 * Fix interactive selection of services ([#4245](#4245)).
github-merge-queue bot pushed a commit that referenced this pull request Nov 20, 2024
### New Features and Improvements

* Add `databricks_mws_network_connectivity_config` and
`databricks_mws_network_connectivity_configs` data source
([#3665](#3665)).
* Add support partitions in policy data sources
([#4181](#4181)).
* Added `databricks_registered_model_versions` data source
([#4100](#4100)).
* Update databricks_permissions resource to support
vector-search-endpoints
([#4209](#4209)).
* add `databricks_serving_endpoints` data source
([#4226](#4226)).


### Bug Fixes

* Add validation for `run_as_mode` in `databricks_query`
([#4233](#4233)).
* Correct handling of updates for empty comments and `force_destroy` in
UC catalog, schema, registered models and volumes
([#4244](#4244)).
* Fix deletion of dashboard if it was trashed out of band
([#4235](#4235)).
* Fix waiting for `databricks_vector_search_index` readiness
([#4243](#4243)).
* Remove single-node validation from interactive clusters
([#4222](#4222)).
* Remove single-node validation from jobs clusters
([#4216](#4216)).
* Use cluster list API to determine pinned cluster status
([#4203](#4203)).
* fix issue cased by setting pause_status in update monitor
([#4242](#4242)).


### Documentation

* Clarify workspace provider config
([#4208](#4208)).
* Update "Databricks Workspace Creator" permissions on gcp-workspace.md
([#4201](#4201)).
* Update `grants.md` references
([#4246](#4246)).
* Update description of `group_id` in
`databricks_mws_ncc_private_endpoint_rule`
([#4238](#4238)).
* remove subnet sharing limitation in AWS
([#4239](#4239)).


### Internal Changes

* Bump Go SDK to latest and generate TF structs
([#4249](#4249)).
* Mark TestUcAccModelServingProvisionedThroughput as flaky. to be rever…
([#4232](#4232)).
* Rename resources directory to products in pluginframework
([#4139](#4139)).
* Revert "mark TestUcAccModelServingProvisionedThroughput as flaky. to …
([#4240](#4240)).
* Set user agent in some resources implemented in plugin framework
([#4187](#4187)).
* make `ApplyAndExpectData` work with nested set
([#4237](#4237)).


### Dependency Updates

* Bump dependencies for Plugin Framework and SDK v2
([#4215](#4215)).
* Bump github.com/hashicorp/hcl/v2 from 2.22.0 to 2.23.0
([#4236](#4236)).
* Bump github.com/hashicorp/terraform-plugin-testing from 1.10.0 to
1.11.0
([#4247](#4247)).


### Exporter

* Add `List` operation for `users` service
([#4204](#4204)).
* Fix interactive selection of services
([#4245](#4245)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants