No way to specify validation constraints on config inputs

[Nicholas Randon]

Example:
ControlVirtualIP:
    Type: OS::Neutron::Port
    Properties:
      name: control_virtual_ip
      network_id: {Ref: NeutronControlPlaneID}
controller0:
    Type: OS::Nova::Server
    Properties:
      image:
        Ref: controllerImage
      image_update_policy:
        Ref: ImageUpdatePolicy
      flavor:
        Ref: OvercloudControlFlavor
      key_name:
        Ref: KeyName
      user_data_format: SOFTWARE_CONFIG
controllerConfig:
    Type: OS::Heat::StructuredConfig
    Properties:
      group: os-apply-config
      config:
         Bobinds: {get_input: VIP}
 controller0Deployment:
    Type: OS::Heat::StructuredDeployment
    Properties:
      config: {Ref: controllerConfig}
      server: {Ref: controller0}
      input_values:
          controller_host:
          Fn::Select:
            - 0
            - Fn::Select:
              - ctlplane
              - Fn::GetAtt:
                - controller0
                - networks

The value VIP is never defined which should be in controller0Deployment and look like this:

         VIP:
          {'Fn::Select': [ip_address, 'Fn::Select': [0, 'Fn::GetAtt': [ControlVirtualIP, fixed_ips]]]}

Currently heat will supply a 'null' json value in it metadata for Bobinds. Heat should detect this and report and error in the template.

[Robert Collins (lifeless)]

More pithily - 'a missing input_value does not give an error'.

[Clint Byrum (clint-fewbar)]

"Fixing" this would be a behavior change unfortunately. I'm not sure we can do that without offering the older, less strict behavior. There are likely template authors relying on it. Consider a scenario where all possible configuration for a particular application is kept in a file, but deployments that don't set anything are used quite often.

So unless people decide that breaking existing templates is o-k, we'll likely have to introduce some kind of "strict_input_values: true" property for the deployment.

[Clint Byrum (clint-fewbar)]

Another option is a new template version which enforces these dependencies.

[Steve Baker (steve-stevebaker)]

You could get into the habit of declaring your inputs rather than implicitly defining them only with a get_input. e.g.

controllerConfig:
    Type: OS::Heat::StructuredConfig
    Properties:
      group: os-apply-config
      inputs:
      - name: VIP
        description: The VIP...
        default: dude_you_forgot
      config:
         Bobinds: {get_input: VIP}

What this inputs schema is missing is the ability to define validation constraints. All you're asking for in this bug is a "required" constraint but it might be nice to reuse our existing parameter constraints.

Constraints would be a welcome feature, but another option for this specific issue is to modify StructuredDeployment so that it raises an error for any get_input which has no corresponding input_values entry. If there are concerns about backwards compatibility then this behaviour could be toggled by a "strict" property on StructuredDeployment. This would be a good task for a new contributor.

[Andrea Rosa (andrea-rosa-m)]

As a really new contributor to Heat I am happy to look at this bug.
Before going ahead we agree that we want to add the "strict_input_values" property?

When we have a consensus on that I'll assign the bug to me and I'll start to work on it.
Please note that It will take a while to me to implement it, depends how steep is the learning curve for Heat ;)

[Steve Baker (steve-stevebaker)]

Here is a guide on how strict_input_values might be implemented:

Add a strict_input_values property to StructuredDeployment
in StructuredDeployment._build_derived_config if strict_input_values then compare the inputs to the supplied input_values property, and raise an error if the input_values is missing something from inputs.

[Andrea Rosa (andrea-rosa-m)]

Steve thanks for the help,. much appreciated.

[Andrea Rosa (andrea-rosa-m)]

While I am working on this change it seems to me that we could implement an different check in the software_config as well.
I can see two different scenario:
1 we want a check to verify that all the input defined in the software_config have a corresponding value in the software_deployment (the case of this bug)
2 add a property to the input schema in the software_config to define an input value as required

At this moment I am working on #1 but I think we need to have the #2 as well.
Am I right or am I missing some important point?
Thanks

[Steve Baker (steve-stevebaker)]

You can already specify a default value in the inputs schema of a SoftwareConfig resource, which I think is what you're describing as #2.

This means that the check in #1 needs to take into account the presence of a default value (which it probably will already if the validation is done in the right place)

[Andrea Rosa (andrea-rosa-m)]

Steve thanks again for your time. I do take into account the presence of default value in my patch, as regards the option I mentioned in the previous comment I was thinking to something slightly different:
we could have an input without a default value but with the Required property set to True, for this value we must have a corresponding input_value.

[Andrea Rosa (andrea-rosa-m)]

Steve I read again your comment and dug into the code and you are right the scenario #2 I described is already covered by defining a default value in the input schema of a software config resource.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/105496

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/105496
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=dd82164bdba430a2ed73be87ca2a90fe7eed06f6
Submitter: Jenkins
Branch: master

commit dd82164bdba430a2ed73be87ca2a90fe7eed06f6
Author: Andrea Rosa <email address hidden>
Date: Tue Jul 8 14:21:10 2014 +0100

    Add validation constraints on config inputs

    This change adds a new property to the StructuredDeployment class to
    enable a check on the config inputs. When this property is set to
    'STRICT' we compare the inputs to the supplied input_values property and
    we raise an exception if the input_values result in any inputs not being
    assigned a value.
    By default this is disabled.
    I did a small refactor of the code adding a couple of extra methods.
    Closes-bug: 1334283

    Change-Id: If132163bcaf4eee61a9499808ab264e7b7f80cb2