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

Refactor the Database Object Permissions guide #41925

Merged
merged 7 commits into from
Jun 3, 2024

Conversation

ptgott
Copy link
Contributor

@ptgott ptgott commented May 22, 2024

Closes #41917

Merge the Database Object Permissions guide into the Database Access RBAC guide for greater discoverability and a clearer division of labor between the two guides.

This change also includes the following edits to make the refactor cleaner, since we can include each troubleshooting step as a separate H3 in the dateabase object permissions H2:

  • Remove an unnecessary troubleshooting step: One step indicates that import rules are validated, which is unnecessary to document, since validation errors are self explanatory.
  • Instead of mentioning the admin user as a troubleshooting step, add a separate H3 for the admin user and describe the admin_user field, which was not mentioned in the original database object permissions guide.

Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@ptgott ptgott requested a review from Tener May 22, 2024 20:07
Copy link

🤖 Vercel preview here: https://docs-ms4ch7j66-goteleport.vercel.app/docs/ver/preview

Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@ptgott ptgott added the no-changelog Indicates that a PR does not require a changelog entry label May 23, 2024
docs/pages/database-access/rbac.mdx Outdated Show resolved Hide resolved
@@ -138,3 +138,213 @@ spec:
db_users: ["{{internal.db_users}}"]
db_names: ["{{internal.db_names}}"]
```

## Database object import rules
Copy link
Contributor

Choose a reason for hiding this comment

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

The old intro part is missing entirely, and the name is gone too.

Database Access Controls is a Teleport feature that lets you configure
role-based access controls for database objects. A database
object
can be a table, view, or stored procedure. With Database Access Controls, you can ensure that users only have permissions to manage the data
they need.

You can define import rules that instruct the Teleport Database Service to
apply labels to database objects imported from databases that match labels configured within the import rule. When a user connects to a database, the Database Service selectively grants permissions by checking database object labels against the user's Teleport roles.

The Database Service grants object-level permissions for the duration of a
connection and revokes them automatically when the connection ends.

I think we should still use a separate name for this feature along with a brief intro section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found it difficult to use the term "Database Access Controls" to describe restrictions on access to database objects, since in the context of this guide, it also seems to refer to the use of roles to restrict access to databases in general. In other words, the "Database Role-Based Access Controls" guide would both describe Database Access Controls, which have to do with database objects, and other role fields that do not.

Would it make sense to include this intro for the entire guide and encompass non-object-related role fields within "Database Access Controls"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is worthwhile to highlight this feature in some way; the "Database Access Controls" name was picked by the product team to that end. @r0mant your thoughts on this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that Database Access Controls is the overall name for the functionality that includes both "old style" database server based and object based levels of granularity. Agree about merging the guides into one, from a user's standpoint, I don't think we want them to think about those as two different features.

Having a some kind of intro that explains that with Database Access Controls you can control access to both entire database servers as well as individual objects within those database servers sounds like a good idea though.

docs/config.json Show resolved Hide resolved
Copy link

🤖 Vercel preview here: https://docs-9felh583r-goteleport.vercel.app/docs/ver/preview

@ptgott
Copy link
Contributor Author

ptgott commented May 29, 2024

@r0mant @Tener I have incorporated your feedback in 9fb105d

Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

lgtm with a couple of notes

docs/pages/database-access/rbac.mdx Outdated Show resolved Hide resolved
docs/pages/database-access/rbac.mdx Outdated Show resolved Hide resolved
@@ -138,3 +155,235 @@ spec:
db_users: ["{{internal.db_users}}"]
db_names: ["{{internal.db_names}}"]
```

## Database object import rules
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add a warning/note somewhere here saying that only Postgres is supported for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to the top of the guide

Copy link

🤖 Vercel preview here: https://docs-ejln21fbb-goteleport.vercel.app/docs/ver/preview

Copy link

🤖 Vercel preview here: https://docs-rl4l3m3w1-goteleport.vercel.app/docs/ver/preview

Copy link

🤖 Vercel preview here: https://docs-cmmm5sca6-goteleport.vercel.app/docs/ver/preview

Copy link

🤖 Vercel preview here: https://docs-p3vqqk0ei-goteleport.vercel.app/docs/ver/preview

Closes #41917

Merge the Database Object Permissions guide into the Database Access
RBAC guide for greater discoverability and a clearer division of labor
between the two guides.

This change also includes the following edits to make the refactor
cleaner, since we can include each troubleshooting step as a separate H3
in the dateabase object permissions H2:

- Remove an unnecessary troubleshooting step: One step indicates that
  import rules are validated, which is unnecessary to document, since
  validation errors are self explanatory.
- Instead of mentioning the admin user as a troubleshooting step, add a
  separate H3 for the admin user and describe the `admin_user` field,
  which was not mentioned in the original database object permissions
  guide.
- Clarify the placing of the `admin_user` field
Per Tener and r0mant feedback, integrate the introduction from the
Database Access Controls page into the newly merged RBAC guide. Frame
Database Access Controls as encompassing both databases and database
objects.
Copy link

🤖 Vercel preview here: https://docs-rghuyapl2-goteleport.vercel.app/docs/ver/preview

Copy link

github-actions bot commented Jun 3, 2024

🤖 Vercel preview here: https://docs-bkjvtlw09-goteleport.vercel.app/docs/ver/preview

@ptgott ptgott added this pull request to the merge queue Jun 3, 2024
Copy link

github-actions bot commented Jun 3, 2024

🤖 Vercel preview here: https://docs-8208jhzjm-goteleport.vercel.app/docs/ver/preview

Merged via the queue into master with commit 39e4c0d Jun 3, 2024
38 checks passed
@ptgott ptgott deleted the paul.gottschling/41917-db branch June 3, 2024 22:12
@public-teleport-github-review-bot

@ptgott See the table below for backport results.

Branch Result
branch/v15 Create PR
branch/v16 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor Database Access Controls docs
4 participants