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

Type check private library data values #296

Closed
pivotaljohn opened this issue Feb 8, 2021 · 6 comments · Fixed by #319 or #322
Closed

Type check private library data values #296

pivotaljohn opened this issue Feb 8, 2021 · 6 comments · Fixed by #319 or #322
Assignees

Comments

@pivotaljohn
Copy link
Contributor

pivotaljohn commented Feb 8, 2021

As a Configuration Consumer using a private library
I want the set of data values supplied to the library to be subject to schema defined within the library.
so that when I misconfigure the library, I get specific, clear, and actionable feedback


Scenario 1: Declarative Schema-Conforming Data Value

Given ytt input that includes a private library
And that private library contains schema
And a data values document targeting the private library that conforms to schema
When I invoke ytt
Then those data values are reflected in the evaluation of the private library

├── _ytt_lib
│   └── fooy
│       ├── schema.yml
│       └── template.yml
├── root.yml
└── values.yml
#! fooy/schema.yml
#@schema/match data_values=True
---
foo: 42
#! fooy/template.yml
#@ load("@ytt:data", "data")
---
foo: #@ data.values.foo
#! root.yml
#@ load("@ytt:template", "template")
#@ load("@ytt:library", "library")
--- #@ template.replace(library.get("fooy").eval())
#! values.yml
#@library/ref "@fooy"
#@data/values
---
foo: 7
$ ytt -f . --enable-experiment-schema
foo: 7

Scenario 2: Declarative Schema-Non-Conforming Data Value

Given ytt input that includes a private library
And that private library contains schema
And a data values document targeting the private library that does NOT conform to schema
When I invoke ytt
Then I see a schema error message in the context of a library evaluation error

#! values.yml
#@library/ref "@fooy"
#@data/values
---
foo: "wrong type"

(all other files are identical from prior scenario)

$ ytt -f .
ytt: Error:
- library.eval: Evaluating library 'fooy':
    in <toplevel>
      root.yml:3 | --- #@ template.replace(library.get("fooy").eval())

    reason:
Overlaying data values (in following order: values.yml):
  Templating file 'values.yml':
values.yml:5 | foo: wrong type
             |
             | TYPE MISMATCH - the value of this item is not what schema expected:
             |      found: string
             |   expected: integer (by schema.yml:4)

Scenario 3: Programmatic Schema-Conforming Data Value

Given ytt input that includes a private library
And that private library contains schema
And a template in the root library supplies a data values document to the private library that conforms to the private library's schema
When I invoke ytt
Then those data values are reflected in the evaluation of the private library

.
├── _ytt_lib
│   └── fooy
│       ├── schema.yml
│       └── template.yml
└── root.yml
#! root.yml
#@ load("@ytt:template", "template")
#@ load("@ytt:library", "library")
---

#@ def dvs_from_root():
#@overlay/match missing_ok=True
foo: from "root" library
#@ end

--- #@ template.replace(library.get("fooy").with_data_values(dvs_from_root()).eval())
#! fooy/schema.yml
#@schema/match data_values=True
---
foo: ""
#! fooy/template.yml
#@ load("@ytt:data", "data")
---
foo: #@ data.values.foo
$ ytt -f . --enable-experiment-schema
foo: from "root" library

Scenario 4: Programmatic Schema-Non-Conforming Data Value

Given ytt input that includes a private library
And that private library contains schema
And a template in the root library supplies a data values document to the private library that does NOT conform to the private library's schema
When I invoke ytt
Then I see a schema error message in the context of a library evaluation error

#! root.yml
#@ load("@ytt:template", "template")
#@ load("@ytt:library", "library")
---

#@ def dvs_from_root():
#@overlay/match missing_ok=True
foo: 13
#@ end

--- #@ template.replace(library.get("fooy").with_data_values(dvs_from_root()).eval())

(all other files are identical from prior scenario)

$ ytt -f .
ytt: Error:
- library.eval: Evaluating library 'fooy':
    in <toplevel>
      root.yml:10 | --- #@ template.replace(library.get("fooy").with_data_values(dvs_from_root()).eval())

    reason:
Overlaying data values (in following order: values.yml):
  Templating file 'values.yml':
root.yml:7 | foo: 13
             |
             | TYPE MISMATCH - the value of this item is not what schema expected:
             |      found: integer
             |   expected: string (by schema.yml:4)

Note: the above error output is inaccurate due to the values.yml reference which is non-existent. We think it would root.yml in this case but haven't verified, yet.

@pivotaljohn pivotaljohn changed the title Type-check private library data values Type check private library data values Feb 9, 2021
@cari-lynn
Copy link
Contributor

cari-lynn commented Mar 9, 2021

  • type check library schema
  • order steps in data values pre-processing so that the data values is being type-checked.
    • when schema is present, make data values overlays more permissive
  • Respect --enable-experiement-schema flag
  • Implement remaining library functions
  • root dir data values that references private lib are checked independently of root dir schema
  • Check that command line supplied dv's targeted at private libraries values are not applied more than once -> the code change identified from this is in a separate story Type Check Command Line Data Values #333.
  • Refactor Schema() function to return a Schema to simplify surrounding logic (may be done in a separate PR)
  • rename schemaEnabled variable in dvpp to something more fitting
  • check if a warning happens more than once when flag is disabled and a schema is in root and a library

Notes on when schema warning/error happens:

When the schema flag is enabled, and a schema is given in the root or in a private library, it is used as the base data values and type checked.
When schema flag is not enabled, then if a schema appears anywhere, the schema is IGNORED and a warning is given. This is because the schema flag is protecting the user against non-GA code.

@cari-lynn
Copy link
Contributor

Created this rough working diagram to map out how data values are processed https://miro.com/app/board/o9J_lRLsKNc=/

@pivotaljohn pivotaljohn self-assigned this Mar 12, 2021
This was linked to pull requests Mar 12, 2021
@cari-lynn cari-lynn reopened this Mar 17, 2021
@github-actions github-actions bot added the carvel triage This issue has not yet been triaged for relevance label Mar 17, 2021
@cari-lynn cari-lynn removed the carvel triage This issue has not yet been triaged for relevance label Mar 19, 2021
@gcheadle-vmware gcheadle-vmware self-assigned this Mar 22, 2021
@cari-lynn
Copy link
Contributor

We split up this issue into three separate PRs

develop 
|_overlay-position       <- PR to address position changes within overlay module (separate for this PR entirely)
|_ refactor-dvpp         <- PR to address refactoring done in data_values_pre_processing.go (must merge prior to this PR)
    |_ private-lib-schema <- this PR

The current focus is to tackle the refactor-dvpp PR first, to unblock private-lib-schema. Then, we can address the remaining feedback for private-lib-schema PR. After all that, then overlay-position.

@cari-lynn
Copy link
Contributor

Reopening this since we are using this work to motivate the changes for #341

@cari-lynn cari-lynn reopened this Apr 8, 2021
@github-actions github-actions bot added the carvel triage This issue has not yet been triaged for relevance label Apr 8, 2021
@cari-lynn cari-lynn removed the carvel triage This issue has not yet been triaged for relevance label Apr 8, 2021
@gcheadle-vmware
Copy link
Contributor

gcheadle-vmware commented Apr 8, 2021

Currently the position of starlark structs does not exist. We will have to do some more thinking about what to do in this case.
cc: @cari-lynn

Update: we just report the position of a right side node so for Starlark structs that have no position, they just have no position reported. Lets tackle this work to add the position separately.

@gcheadle-vmware
Copy link
Contributor

ytt release v0.32.0 cut, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants