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

Include a permissions section in all templates #1713

Merged

Conversation

lennartkats-db
Copy link
Contributor

@lennartkats-db lennartkats-db commented Aug 22, 2024

Changes

This updates the templates to include a permissions section. Having a permissions section is a best practice, is helpful to understand the notion of permissions, and helps diagnose permission errors (#1386).

This is a cherry-pick from #1387.

This change was verified to work both in dev and prod. Existing unit tests validate the validity of the templates in these modes.

@lennartkats-db lennartkats-db marked this pull request as ready for review August 23, 2024 09:53
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

IIUC this doesn't change anything functionally, correct?

@lennartkats-db
Copy link
Contributor Author

Yeah, no functional change. If Alice deploys, then Alice is still the OWNER like before, even though there's a permission that says she has CAN_MANAGE. If the template is freshly created from the template and Bob deploys, Bob would always get an error about run_as Alice and/or deployment to Users/Alice.

@lennartkats-db lennartkats-db added this pull request to the merge queue Sep 3, 2024
Merged via the queue into databricks:main with commit 072fa81 Sep 3, 2024
5 checks passed
@lennartkats-db lennartkats-db deleted the permissions-in-templates branch September 3, 2024 07:59
andrewnester added a commit that referenced this pull request Sep 5, 2024
CLI:
 * Do not error if we cannot prompt for a profile in `auth login` ([#1745](#1745)).

Bundles:
 * Pass along $AZURE_CONFIG_FILE to Terraform process ([#1734](#1734)).
 * Add prompt when a pipeline recreation happens ([#1672](#1672)).
 * Use materialized views in the default-sql template ([#1709](#1709)).
 * Update templates to latest LTS DBR ([#1715](#1715)).
 * Make lock optional in the JSON schema ([#1738](#1738)).
 * Do not suppress normalisation diagnostics for resolving variables ([#1740](#1740)).
 * Include a permissions section in all templates ([#1713](#1713)).
 * Fixed complex variables are not being correctly merged from include files ([#1746](#1746)).
 * Fixed variable override in target with full variable syntax ([#1749](#1749)).

Internal:
 * Consider serverless clusters as compatible for Python wheel tasks ([#1733](#1733)).
 * PythonMutator: explain missing package error ([#1736](#1736)).
 * Add `dyn.Time` to box a timestamp with its original string value ([#1732](#1732)).
 * Fix streaming of stdout, stdin, stderr in cobra test runner ([#1742](#1742)).

Dependency updates:
 * Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0 ([#1741](#1741)).
github-merge-queue bot pushed a commit that referenced this pull request Sep 5, 2024
CLI:
* Do not error if we cannot prompt for a profile in `auth login`
([#1745](#1745)).

Bundles:

As of this release CLI will show a prompt is if there are configuration
changes which will lead to a DLT recreation.
Users can skip the prompt by specifying the `--auto-approve` flag

* Pass along $AZURE_CONFIG_FILE to Terraform process
([#1734](#1734)).
* Add prompt when a pipeline recreation happens
([#1672](#1672)).
* Use materialized views in the default-sql template
([#1709](#1709)).
* Update templates to latest LTS DBR
([#1715](#1715)).
* Make lock optional in the JSON schema
([#1738](#1738)).
* Do not suppress normalisation diagnostics for resolving variables
([#1740](#1740)).
* Include a permissions section in all templates
([#1713](#1713)).
* Fixed complex variables are not being correctly merged from include
files ([#1746](#1746)).
* Fixed variable override in target with full variable syntax
([#1749](#1749)).

Internal:
* Consider serverless clusters as compatible for Python wheel tasks
([#1733](#1733)).
* PythonMutator: explain missing package error
([#1736](#1736)).
* Add `dyn.Time` to box a timestamp with its original string value
([#1732](#1732)).
* Fix streaming of stdout, stdin, stderr in cobra test runner
([#1742](#1742)).

Dependency updates:
* Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0
([#1741](#1741)).

---------

Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants