Skip to content

Commit 7498df4

Browse files
denikdenik
authored
direct: grants: revoke removed principals + pass PlanEntry to DoUpdate (#4824)
## Changes - When a principal is removed from grants config, deploy now revokes their privileges instead of silently leaving them in place. This matches terraform behaviour. - `DoUpdate` interface changed to receive `*PlanEntry` instead of `Changes`, giving resources access to remote state directly. ## Why New behaviour matches terraform's. Grants API is incremental, it only replaces principals mentioned in the payload and leaves other one alone (instead of wiping them, which is the incorrect assumption previous impl was built upon). Passing PlanEntry instead of just Changes simplifies grants update logic instead of parsing Changes paths like (`[principal='bob'].privileges[0]`), it compares desired state against `PlanEntry.RemoteState`. A few other resources had a need to view remote state as well, so it's generally useful. ## Tests - New acceptance tests cover the remove-principal and out-of-band-principal drift scenarios. - gron.py got new option --noindex which prints array elements as `element[]` instead of `element[1]` which allows to ignore order differences. - Updates to testserver for grants & schemas to match AWS backend better to enable local+cloud test.
1 parent adfdee6 commit 7498df4

58 files changed

Lines changed: 589 additions & 119 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎NEXT_CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* engine/direct: Fix 400 error when deploying grants with ALL_PRIVILEGES ([#4801](https://github.com/databricks/cli/pull/4801))
1212
* Deduplicate grant entries with duplicate principals or privileges during initialization ([#4801](https://github.com/databricks/cli/pull/4801))
1313
* engine/direct: Fix unwanted recreation of secret scopes when scope_backend_type is not set ([#4834](https://github.com/databricks/cli/pull/4834))
14+
* engine/direct: Fix deploying removed principals ([#4824](https://github.com/databricks/cli/pull/4824))
1415

1516
### Dependency updates
1617

‎acceptance/bin/gron.py‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#!/usr/bin/env python3
2+
import argparse
23
import json
34
import sys
45
from pathlib import Path
@@ -7,7 +8,7 @@
78
from print_requests import read_json_many
89

910

10-
def gron(obj, path="json"):
11+
def gron(obj, path="json", noindex=False):
1112
"""Flatten JSON into greppable assignments.
1213
1314
The path parameter defaults to "json" to match the original gron tool,
@@ -26,6 +27,10 @@ def gron(obj, path="json"):
2627
json.items[0] = "apple";
2728
json.items[1] = "banana";
2829
30+
>>> gron({"items": ["apple", "banana"]}, noindex=True)
31+
json.items[] = "apple";
32+
json.items[] = "banana";
33+
2934
>>> gron({"tasks": [{"libraries": [{"whl": "file.whl"}]}]})
3035
json.tasks[0].libraries[0].whl = "file.whl";
3136
@@ -38,31 +43,34 @@ def gron(obj, path="json"):
3843
print(f"{path} = {{}};")
3944
else:
4045
for key in obj:
41-
gron(obj[key], f"{path}.{key}")
46+
gron(obj[key], f"{path}.{key}", noindex=noindex)
4247
elif isinstance(obj, list):
4348
if not obj:
4449
print(f"{path} = [];")
4550
else:
4651
for i, item in enumerate(obj):
47-
gron(item, f"{path}[{i}]")
52+
index = "[]" if noindex else f"[{i}]"
53+
gron(item, f"{path}{index}", noindex=noindex)
4854
else:
4955
print(f"{path} = {json.dumps(obj)};")
5056

5157

5258
def main():
53-
if len(sys.argv) > 1:
54-
with open(sys.argv[1]) as f:
55-
content = f.read()
56-
data = read_json_many(content)
57-
if len(data) == 1:
58-
data = data[0]
59+
parser = argparse.ArgumentParser()
60+
parser.add_argument("--noindex", action="store_true")
61+
parser.add_argument("file", nargs="?")
62+
args = parser.parse_args()
63+
64+
if args.file:
65+
content = Path(args.file).read_text()
5966
else:
6067
content = sys.stdin.read()
61-
data = read_json_many(content)
62-
if len(data) == 1:
63-
data = data[0]
6468

65-
gron(data)
69+
data = read_json_many(content)
70+
if len(data) == 1:
71+
data = data[0]
72+
73+
gron(data, noindex=args.noindex)
6674

6775

6876
if __name__ == "__main__":

‎acceptance/bundle/migrate/grants/out.original_state.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,15 @@
180180
"attributes": {
181181
"catalog_name": "main",
182182
"comment": null,
183-
"enable_predictive_optimization": null,
183+
"enable_predictive_optimization": "INHERIT",
184184
"force_destroy": true,
185185
"id": "main.schema_grants",
186-
"metastore_id": null,
186+
"metastore_id": "[UUID]",
187187
"name": "schema_grants",
188188
"owner": "[USERNAME]",
189189
"properties": null,
190190
"provider_config": [],
191-
"schema_id": "",
191+
"schema_id": "[UUID]",
192192
"storage_root": null
193193
},
194194
"sensitive_attributes": [],

‎acceptance/bundle/resources/grants/schemas/duplicate_principals/out.plan.direct.json‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,18 @@
1212
"catalog_type": "MANAGED_CATALOG",
1313
"created_at": [UNIX_TIME_MILLIS][0],
1414
"created_by": "[USERNAME]",
15+
"effective_predictive_optimization_flag": {
16+
"inherited_from_name": "deco-uc-prod-isolated-aws-us-east-1",
17+
"inherited_from_type": "METASTORE",
18+
"value": "ENABLE"
19+
},
20+
"enable_predictive_optimization": "INHERIT",
1521
"full_name": "main.schema_dup_grants_[UNIQUE_NAME]",
22+
"metastore_id": "[UUID]",
1623
"name": "schema_dup_grants_[UNIQUE_NAME]",
1724
"owner": "[USERNAME]",
18-
"updated_at": [UNIX_TIME_MILLIS][0],
25+
"schema_id": "[UUID]",
26+
"updated_at": [UNIX_TIME_MILLIS][1],
1927
"updated_by": "[USERNAME]"
2028
}
2129
},

‎acceptance/bundle/resources/grants/schemas/out_of_band_principal/databricks.yml.tmpl‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
bundle:
2+
name: schema-grants-out-of-band-principal-$UNIQUE_NAME
3+
4+
resources:
5+
schemas:
6+
grants_schema:
7+
name: schema_out_of_band_principal_$UNIQUE_NAME
8+
catalog_name: main
9+
grants:
10+
- principal: $CURRENT_USER_NAME
11+
privileges:
12+
- CREATE_TABLE

‎acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan.direct.json‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
{
2+
"plan_version": 2,
3+
"cli_version": "[DEV_VERSION]",
4+
"lineage": "[UUID]",
5+
"serial": 1,
6+
"plan": {
7+
"resources.schemas.grants_schema": {
8+
"action": "skip",
9+
"remote_state": {
10+
"browse_only": false,
11+
"catalog_name": "main",
12+
"catalog_type": "MANAGED_CATALOG",
13+
"created_at": [UNIX_TIME_MILLIS][0],
14+
"created_by": "[USERNAME]",
15+
"effective_predictive_optimization_flag": {
16+
"inherited_from_name": "deco-uc-prod-isolated-aws-us-east-1",
17+
"inherited_from_type": "METASTORE",
18+
"value": "ENABLE"
19+
},
20+
"enable_predictive_optimization": "INHERIT",
21+
"full_name": "main.schema_out_of_band_principal_[UNIQUE_NAME]",
22+
"metastore_id": "[UUID]",
23+
"name": "schema_out_of_band_principal_[UNIQUE_NAME]",
24+
"owner": "[USERNAME]",
25+
"schema_id": "[UUID]",
26+
"updated_at": [UNIX_TIME_MILLIS][1],
27+
"updated_by": "[USERNAME]"
28+
}
29+
},
30+
"resources.schemas.grants_schema.grants": {
31+
"depends_on": [
32+
{
33+
"node": "resources.schemas.grants_schema",
34+
"label": "${resources.schemas.grants_schema.id}"
35+
}
36+
],
37+
"action": "update",
38+
"new_state": {
39+
"value": {
40+
"securable_type": "schema",
41+
"full_name": "main.schema_out_of_band_principal_[UNIQUE_NAME]",
42+
"__embed__": [
43+
{
44+
"principal": "[USERNAME]",
45+
"privileges": [
46+
"CREATE_TABLE"
47+
]
48+
}
49+
]
50+
}
51+
},
52+
"remote_state": {
53+
"securable_type": "schema",
54+
"full_name": "main.schema_out_of_band_principal_[UNIQUE_NAME]",
55+
"__embed__": [
56+
{
57+
"principal": "[USERNAME]",
58+
"privileges": [
59+
"CREATE_TABLE"
60+
]
61+
},
62+
{
63+
"principal": "deco-test-user@databricks.com",
64+
"privileges": [
65+
"USE_SCHEMA"
66+
]
67+
}
68+
]
69+
},
70+
"changes": {
71+
"[principal='deco-test-user@databricks.com']": {
72+
"action": "update",
73+
"remote": {
74+
"principal": "deco-test-user@databricks.com",
75+
"privileges": [
76+
"USE_SCHEMA"
77+
]
78+
}
79+
}
80+
}
81+
}
82+
}
83+
}

‎acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan.terraform.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"cli_version": "[DEV_VERSION]",
3+
"plan": {
4+
"resources.schemas.grants_schema": {
5+
"action": "skip"
6+
},
7+
"resources.schemas.grants_schema.grants": {
8+
"action": "update"
9+
}
10+
}
11+
}

‎acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan2.direct.json‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
{
2+
"plan_version": 2,
3+
"cli_version": "[DEV_VERSION]",
4+
"lineage": "[UUID]",
5+
"serial": 2,
6+
"plan": {
7+
"resources.schemas.grants_schema": {
8+
"action": "skip",
9+
"remote_state": {
10+
"browse_only": false,
11+
"catalog_name": "main",
12+
"catalog_type": "MANAGED_CATALOG",
13+
"created_at": [UNIX_TIME_MILLIS][0],
14+
"created_by": "[USERNAME]",
15+
"effective_predictive_optimization_flag": {
16+
"inherited_from_name": "deco-uc-prod-isolated-aws-us-east-1",
17+
"inherited_from_type": "METASTORE",
18+
"value": "ENABLE"
19+
},
20+
"enable_predictive_optimization": "INHERIT",
21+
"full_name": "main.schema_out_of_band_principal_[UNIQUE_NAME]",
22+
"metastore_id": "[UUID]",
23+
"name": "schema_out_of_band_principal_[UNIQUE_NAME]",
24+
"owner": "[USERNAME]",
25+
"schema_id": "[UUID]",
26+
"updated_at": [UNIX_TIME_MILLIS][1],
27+
"updated_by": "[USERNAME]"
28+
}
29+
},
30+
"resources.schemas.grants_schema.grants": {
31+
"depends_on": [
32+
{
33+
"node": "resources.schemas.grants_schema",
34+
"label": "${resources.schemas.grants_schema.id}"
35+
}
36+
],
37+
"action": "skip",
38+
"remote_state": {
39+
"securable_type": "schema",
40+
"full_name": "main.schema_out_of_band_principal_[UNIQUE_NAME]",
41+
"__embed__": [
42+
{
43+
"principal": "[USERNAME]",
44+
"privileges": [
45+
"CREATE_TABLE"
46+
]
47+
}
48+
]
49+
}
50+
}
51+
}
52+
}

‎acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan2.terraform.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"cli_version": "[DEV_VERSION]",
3+
"plan": {
4+
"resources.schemas.grants_schema": {
5+
"action": "skip"
6+
},
7+
"resources.schemas.grants_schema.grants": {
8+
"action": "skip"
9+
}
10+
}
11+
}

‎acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.test.toml‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)