Ensure owner/mgmt permission for current user #3780
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
12 failing tests:
|
denik
changed the title
denik
force-pushed
the
denik/current-user-permissions
branch
from
b2f59df to
bb5413f
Compare
denik
requested review from
andrewnester,
anton-107,
pietern and
shreyas-goenka
as code owners
andrewnester
approved these changes
Oct 20, 2025
libs/dyn/visit_get.go
Outdated
| } | ||
|
|
||
| // Get1 returns the value without the error (InvalidValue indicates that error did happen) | ||
| func Get1(v Value, path string) Value { |
Contributor
There was a problem hiding this comment.
Can we call it GetValue?
| priorityPermission := mgmtPerms[0] | ||
| permissionToUpgrade := -1 | ||
|
|
||
| permissionArray := permissions.MustSequence() |
Contributor
There was a problem hiding this comment.
Shouldn't we use AsSequence instead and retirn error if it fails to convert?
Contributor
Author
There was a problem hiding this comment.
make sense; although we don't need an error, we can just skip it: 8f7a965
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 20, 2025
## Why Going to make some changes there, want to record how requests look like and how they will change. #3780
denik
force-pushed
the
denik/current-user-permissions
branch
from
730857b to
c60359b
Compare
lennartkats-db
added a commit
that referenced
this pull request
Oct 21, 2025
After merging main, the acceptance tests now reflect the new permissions behavior: - Automatically adds IS_OWNER permission for current user - This matches the changes from PR #3780
deco-sdk-tagging bot
added a commit
that referenced
this pull request
Oct 22, 2025
## Release v0.274.0 ### Bundles * Fix a panic in TF when it fails to read the job ([#3799](#3799)) * For secret scopes, no longer remove current user's permissions ([#3780](#3780)) * Automatically add owner permissions during bundle initialization, this makes final permissions visible in 'bundle validate -o json' ([#3780](#3780))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Instead of removing current user from permissions and relying on terraform to add it back, we'll instead add IS_OWNER/CAN_MANAGE ourselves.
Previous attempt to remove this mutator completely #3688 failed because backend complains about "ambiguous" permissions when both CAN_MANAGE and IS_OWNER are present. Thus we do additional transformation here: we upgrade CAN_MANAGE to IS_OWNER if we can.
Don't apply this logic to secret scopes resource as it's not implemented via databricks_permissions resource in terraform and does not have IS_OWNER/CAN_MANAGE insertion logic. This means we no longer filter out current user permissions from secret scopes resources.
Why
Tests
#3781