Skip to content

Add extra database validations to CreateDatabase#18776

Merged
greedy52 merged 9 commits intomasterfrom
STeve/create_database_api_validation
Nov 25, 2022
Merged

Add extra database validations to CreateDatabase#18776
greedy52 merged 9 commits intomasterfrom
STeve/create_database_api_validation

Conversation

@greedy52
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 commented Nov 24, 2022

Related issue #17454

Why
When creating databases through static config, we have a bunch of awesome pre-checks to validate the inputs. It would be nice to have the same checks when creating database through the webapi (or tctl)

Changes

  • Moved redis.ParseRedisAddress from lib/srv/db/redis/connection.go its own package lib/srv/db/redis/connection
  • Moved database validation from service.Database.CheckAndSetDefaults to services.ValidateDatabase (and one check to api.DatabaseV3.CheckAndSetDefaults)
  • Added validation to services/local/databases.go similar to other validations in services/local

@greedy52 greedy52 added database-access Database access related issues and PRs discover Issues related to Teleport Discover labels Nov 24, 2022
@greedy52 greedy52 self-assigned this Nov 24, 2022
@greedy52 greedy52 changed the title Add database validation for create-database web api Add extra database validations to CreateDatabase Nov 24, 2022
// CreateDatabase creates a new database resource.
func (s *DatabaseService) CreateDatabase(ctx context.Context, database types.Database) error {
if err := database.CheckAndSetDefaults(); err != nil {
if err := services.ValidateDatabase(database); err != nil {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the only real change, rest are moving things around.

@greedy52 greedy52 marked this pull request as ready for review November 24, 2022 16:40
@github-actions github-actions Bot requested review from LKozlowski and zmb3 November 24, 2022 16:40
Comment thread lib/service/cfg.go Outdated
Comment thread lib/service/cfg.go Outdated
@greedy52 greedy52 enabled auto-merge (squash) November 25, 2022 14:09
@greedy52
Copy link
Copy Markdown
Contributor Author

Thanks for the quick review 😄

@greedy52 greedy52 merged commit 72b5a2f into master Nov 25, 2022
@github-actions
Copy link
Copy Markdown
Contributor

@greedy52 See the table below for backport results.

Branch Result
branch/v11 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database-access Database access related issues and PRs discover Issues related to Teleport Discover

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants