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

Normalize prefix handling in blueprints #1003

Merged
merged 11 commits into from
Nov 23, 2022
Merged

Normalize prefix handling in blueprints #1003

merged 11 commits into from
Nov 23, 2022

Conversation

kunzese
Copy link
Collaborator

@kunzese kunzese commented Nov 22, 2022

This PR normalizes the prefix handling in blueprints, similar to #964.

Fixes #967

@kunzese
Copy link
Collaborator Author

kunzese commented Nov 22, 2022

@skalolazka FYI

@juliocc
Copy link
Collaborator

juliocc commented Nov 22, 2022

Thanks @kunzese!

I'm not sure if we want the default=null for blueprints. @ludoo wdyt?

@skalolazka
Copy link
Member

OMG, Sebastian, this is awesome! Thanks for taking your time!
Nit: we did change "can not" to "cannot" in modules/ %)

@ludoo
Copy link
Collaborator

ludoo commented Nov 22, 2022

Thanks @kunzese!

I'm not sure if we want the default=null for blueprints. @ludoo wdyt?

true, blueprints could require the prefix to be passed in, so as not to default to existing project/bucket names

@kunzese
Copy link
Collaborator Author

kunzese commented Nov 22, 2022

OMG, Sebastian, this is awesome! Thanks for taking your time! Nit: we did change "can not" to "cannot" in modules/ %)

But not in the ./CONTRIBUTING.md where i copied it from :p but i adjusted it everywhere.

@kunzese
Copy link
Collaborator Author

kunzese commented Nov 23, 2022

@ludoo @juliocc what if we set a reasonable default value for prefix?

variable "prefix" {
  description = "Optional prefix used for resource names."
  type        = string
  default     = "blueprint" // demo or fast or example or whatever
  validation {
    condition     = var.prefix != ""
    error_message = "Prefix cannot be empty, please use null instead."
  }
}

This way everything has a prefix per default, the user does not have to supply one when they don't need, but we keep the possible to just not use any prefix if someone decides to do so.

@ludoo
Copy link
Collaborator

ludoo commented Nov 23, 2022

@ludoo @juliocc what if we set a reasonable default value for prefix?

variable "prefix" {
  description = "Optional prefix used for resource names."
  type        = string
  default     = "blueprint" // demo or fast or example or whatever
  validation {
    condition     = var.prefix != ""
    error_message = "Prefix cannot be empty, please use null instead."
  }
}

This way everything has a prefix per default, the user does not have to supply one when they don't need, but we keep the possible to just not use any prefix if someone decides to do so.

The problem for blueprints (where there is no org-wide naming convention with a unique token, usually at the beginning of the name) is that setting a default prefix will originate clashes for each user after the first one that runs apply on project ids and GCS bucker names, which are unique across GCP.

For blueprints we should really not set a default for prefix, and ask users to set it in a tfvars file or pass it in via environment or arg.

@kunzese
Copy link
Collaborator Author

kunzese commented Nov 23, 2022

Reworked the prefix handling, so that it is now mandatory.

@kunzese kunzese merged commit e4fc47a into master Nov 23, 2022
@kunzese kunzese deleted the kunzese/issue-967 branch November 23, 2022 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefix handling in blueprints
4 participants