taskcat icon indicating copy to clipboard operation
taskcat copied to clipboard

PROPOSAL - per-test/per-region parameters

Open jaymccon opened this issue 5 years ago • 10 comments

At present parameters defined in the global config, used for overriding parameters defined in tests/projects, is a simple key-value representation of parameter names and values. where ever the name matches a parameter, it is overridden. This does not allow for providing per-test or per-region overrides. An example of this would be where a Quick Start requires a region specific ID as a parameter value.

The proposed change has two components:

  1. Allow nested parameters in parameters under project and tests. If the value of a key is a dict, and the key is a region name, then the parameters defined under it are only applied to that region. For example:
tests:
  some-test:
    parameters:
      eu-west-2:
        myParam: some-value-for-this-region
      myParam: default-value

This could alternatively be done by allowing sub-keys on the region lists instead of the parameters.

  1. Create a new pseudo parameter that points to an override key. For example:
tests:
  some-test:
    parameters:
      myParam: $[override_myVal]

in the global config we would have myVal defined, and the value for that would be substituted.

general:
  parameters:
    myVal: this-value-will-go-to-myParam

with these two changes we would be able to do complex overrides, of potentially secret values:

project:
  parameters:
    myParam: $[override_defaultVal]
tests:
  some-test:
    parameters:
      eu-west-2:
        myParam: $[override_regionVal]
      myParam: $[override_testVal]

Thoughts ?

jaymccon avatar Feb 12 '20 18:02 jaymccon

thinking about this a bit more, I prefer making 1. use the regions list. Provides slightly more flexibility, and means we don't need to deal with complicating the validation on parameter keys being either a literal key or a region name.

jaymccon avatar Feb 12 '20 18:02 jaymccon

Is this a blocker, or nice-to-have?

That aside, this appears as a breaking change at first glance, since we assume key:value under parameters... Will we introduce backwards compatibility ?

Assuming we will introduce backwards compat, this kinda-sorta goes against the grain of taskcat. In my mind, taskcat is a tool that tests the same configuration (test definition) across multiple regions. I'm all for flexibility, but I'm concerned about corner cases, especially in CICD pipelines.

andrew-glenn avatar Feb 12 '20 18:02 andrew-glenn

It's a blocker to be able to run tests in ci for connect integrations (they require a parameter with an id of an already existing connect instance in the same region). It's a nice to have for me because I have other use cases where it will simplify my life.

It's not a breaking change.

jaymccon avatar Feb 12 '20 18:02 jaymccon

It's not a breaking change.

Without additional logic, it is. Current test definitions expect..

parameters:
  key: value

Not

parameters:
  region_name:
    key:value

We'll need to add something like..

if isinstance(value, dict):
   (...)
elif isinstance(value, str):
   (...)

andrew-glenn avatar Feb 12 '20 18:02 andrew-glenn

since we assume key:value under parameters

parameters can be any type except dict at present (cfn doesn't accept dict). All this will introduce is that IF a parameter is of type dict, it is treated specially. At present taskcat will just crash if you give it a dict.

jaymccon avatar Feb 12 '20 18:02 jaymccon

since we assume key:value under parameters

parameters can be any type except dict at present (cfn doesn't accept dict). All this will introduce is that IF a parameter is of type dict, it is treated specially. At present taskcat will just crash if you give it a dict.

More ☕️ needed. Happy to review a PR when it's ready.

andrew-glenn avatar Feb 12 '20 18:02 andrew-glenn

I'm thinking it would be best to put the regional params under regions instead. This way we can do per region overrides for all tests (put it in project) or for a specific test (put it in test).

Example:

project:
  regions:
    - us-east-1:
        myParam: ue1val
    - ca-central-1
tests:
  basic-test:
    parameters:
      myParam: basic-test-default
  override-test:
    regions:
      - us-west-2:
          myParam: override-test-uw2val
      - us-east-1:
          myParam: override-test-ue1val
      - ca-central-1
  parameters:
    myParam: override-test-default

would result in the following stacks/parameters:

basic-test - us-east-1 - myParam: ue1val basic-test - ca-central-1 - myParam: basic-test-default override-test - us-west-2 - myParam: override-test-uw2val override-test - us-east-1 - myParam: override-test-ue1val override-test - ca-central-1 - myParam: basic-test-default

@avattathil @andrew-glenn thoughts ?

jaymccon avatar Feb 12 '20 19:02 jaymccon

@jaymccon

I'm good with this. It just means a few more calls to the template_params stuff. Would it make sense to craft a list of parameter sources, like what's done in the Config object? Merge the dictionaries for each region, then pass it along for conversion? I want to make sure we nail down inheritance without any complications.

Global Overrides -> Project Overrides -> RegionParams -> TestParams -> ProjectParams -> Final dictionary -> psuedo-param conversion -> launch stack?

andrew-glenn avatar Feb 12 '20 19:02 andrew-glenn

Just to circle back - I'm running into a situation where this would mitigate pain. Any questions on the value-add I had are now gone.

andrew-glenn avatar Mar 17 '20 14:03 andrew-glenn

This would be awesome for us as well. We ship out CF stacks to customers to deploy our software so we allow them to choose the VPC, Subnets, etc so for testing in different regions we would need these to be per-region.

zetas avatar Apr 06 '20 03:04 zetas