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

BREAKING CHANGE: Pass SLO config and Error Budget Policy paths instead of content #56

Closed
wants to merge 18 commits into from

Conversation

ocervell
Copy link

@ocervell ocervell commented Oct 19, 2020

Pass SLO configs and Error Budget Policy file paths instead of content to go around TF variable limitation.
Because of #55 (cannot have complex variables), the module has to take in config file paths instead of decoded configs to support the latest added attribute metrics.

Also adds:

  • Custom backends / exporters (dynamic loading) supported in v1.3.x
  • Variable replacements within the module to simplify end user configs

This will also help with updates to the SLO config or error budget policy JSON schemas without needing to update the Terraform module inputs.

Fix #55
Fix #57

@ocervell ocervell changed the title Upgrade to slo-generator v1.3.0 BREAKING CHANGE: Upgrade to slo-generator v1.3.0 Oct 19, 2020
@ocervell ocervell requested a review from morgante October 20, 2020 10:39
@ocervell ocervell changed the title BREAKING CHANGE: Upgrade to slo-generator v1.3.0 BREAKING CHANGE: Pass SLO config and Error Budget Policy paths instead of content Oct 23, 2020
@apparentlymart
Copy link

apparentlymart commented Oct 29, 2020

Hi @ocervell,

I got here via some references to issues in the hashicorp/terraform repository. Sorry for showing up out of nowhere, but I just wanted to offer an alternative approach that I think would avoid the breaking change aspect of this and leave you open to make use of the likely future feature of optional attributes in object types.

I see in the current version that you have a few variables defined as type = list(any), which as you've seen means list of any single type, where any is a placeholder for a type to be inferred automatically similar to List<T> in some other languages.

A different way to get the result I think you were looking for (where the items in the sequence can each have a separate type) would be to change the declaration to type = any and then represent the constraint that it must be a sequence type (either a list or a tuple) using a validation rule something like this:

variable "example" {
  type = any

  validation {
    condition     = can(var.example[0]) # numeric indexing is possible only for sequences
    error_message = "Must be a list of objects."
  }

  validation {
    # This expression is finicky; Terraform 0.14 has a new "alltrue"
    # function that makes this easier but I assume you want to
    # keep supporting Terraform 0.13, so I'm showing the finicky
    # way.
    condition     = [for v in var.example : true if can(v.required_attr)] == length(var.example)
    error_message = "Must be a list of objects where each object has a required_attr attribute."
  }

  # Potentially other validation rules for the types of different
  # attributes, if you like.
}

If the optional attribute experiment gets good feedback during the 0.14 series and stablizes as-is in 0.15 then, assuming you were then willing to require Terraform 0.15 for some new release, this could then be simplified back to a type = object({ ... }) constraint with some of the attributes marked as optional, assuming that you wanted to have an explicit object type constraint rather than just pass arbitrary caller data up to the remote system.

@ocervell
Copy link
Author

ocervell commented Nov 2, 2020

@apparentlymart thanks for helping here, much appreciated !

Would type=any support any complex type passed to it ? Such as list of nested maps with optional arguments ?

I'm interested in passing a list of maps (serialised from the yaml below) directly to the module config variable:

- class: Prometheus
  url: <URL>
  metrics:
   - name: test
      alias: ok
      additional_labels: [test1, test2, test3]
   - name: test2
      alias: ok2
- class: Stackdriver
  project_id: <PROJECT_ID>

Also, could we imagine using the validation blocks below, build something schema-based to validate all inputs ? Like jsonschema embedded in TF.

@ocervell
Copy link
Author

ocervell commented Nov 3, 2020

@morgante I will close this for now since the breaking change is not needed based on the workaround shown above.

@ocervell ocervell closed this Nov 3, 2020
@ocervell ocervell deleted the slo-generator-v1.3.0-new branch November 5, 2020 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants