charm-build should error if defining options for a non-existant layer #173

Closed
chuckbutler opened this Issue Apr 4, 2016 · 1 comment

Comments

Projects
None yet
2 participants
Contributor

chuckbutler commented Apr 4, 2016

When defining layer options according to JSON Schema, such as the following link describes: i would expect any inheriting layer, that defines an option that doesn't exist in the list to error during charm build

Given in the base layer:

defines:
  storage-driver:
    enum: [overlay, aufs, btrfs, zfs]
    default: aufs
    description: |
      Used in conjunction with the storage volume in metadata.yaml.
      Supported providers, when prompted, will allow you to mount a block
      device, formatted for use with the storage driver configured here.

And in the inheriting layer:

options:
  storage-driver:
    value: schenanigans

I expected during the build time, the base schema to be validated. ENUM type should have limited the values to those defined as valid: 'aufs', 'btrfs', 'zfs', or 'overlay' - anything outside of this pre-defined list when provided as a value should have actively prevented the build, or failed it, as it doesn't validate against the defined schema.

@marcoceppi marcoceppi added this to the 2.NEXT milestone Apr 4, 2016

Contributor

chuckbutler commented Apr 4, 2016

It appears I input the options wrong which raises a different issue. The layers options are not validating if we are even targeting an existing layer being used during the build process.

the inheriting layer should have appeared as the following:

options:
  docker:
    storage-driver: schenanigans

And the providing layer to trigger was updated to:

  storage-driver:
    type: string
    enum: ['overlay', 'aufs', 'btrfs', 'zfs']
    default: aufs
    description: |
      Used in conjunction with the storage volume in metadata.yaml.
      Supported providers, when prompted, will allow you to mount a block
      device, formatted for use with the storage driver configured here.

The resulting output:

charmtools.build.tactics: Invalid value for option docker.storage-driver: 'poop' is not one of ['overlay', 'aufs', 'btrfs', 'zfs']

@chuckbutler chuckbutler changed the title from json schema validation doesn't appear to be validating to charm build needs to validate if the user has targeted a valid layer being used in the build Apr 4, 2016

@chuckbutler chuckbutler changed the title from charm build needs to validate if the user has targeted a valid layer being used in the build to charm build needs to validate if the user has targeted a valid layer(s options) being used in the build Apr 4, 2016

@chuckbutler chuckbutler changed the title from charm build needs to validate if the user has targeted a valid layer(s options) being used in the build to charm-build should error if defining options for a non-existant layer Apr 4, 2016

johnsca added a commit to johnsca/charm-tools that referenced this issue Apr 4, 2016

Validate layers for options (fixes #173)
Ensure that options are only set for layers that are included.
This also ensures that one doesn't forget to nest the options under the
layer name.

Note: The `additionalProperties` is actually sufficient, but produces a
somewhat obscure error message.

@marcoceppi marcoceppi closed this in 134e860 Apr 4, 2016

marcoceppi added a commit that referenced this issue Apr 4, 2016

Merge pull request #175 from johnsca/issue-173
Validate layers for options (fixes #173)

mbruzek added a commit to mbruzek/charm-tools that referenced this issue Apr 11, 2016

Validate layers for options (fixes #173)
Ensure that options are only set for layers that are included.
This also ensures that one doesn't forget to nest the options under the
layer name.

Note: The `additionalProperties` is actually sufficient, but produces a
somewhat obscure error message.

@marcoceppi marcoceppi modified the milestones: 2.1.PATCH, 2.NEXT Apr 19, 2016

marcoceppi added a commit that referenced this issue Apr 19, 2016

Validate layers for options (fixes #173)
Ensure that options are only set for layers that are included.
This also ensures that one doesn't forget to nest the options under the
layer name.

Note: The `additionalProperties` is actually sufficient, but produces a
somewhat obscure error message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment