-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add schemaVersion to porter.yaml #1946
Add schemaVersion to porter.yaml #1946
Conversation
/azp run porter-integration |
Pull request contains merge conflicts. |
def2ab2
to
0c74d27
Compare
This adds a document schema version to the Porter manifest (porter.yaml) similar to what we already have on credential and parameter sets. The schema of the any document should not change within a major version. I've added a page that lists which schema versions are used by various porter versions. Signed-off-by: Carolyn Van Slyck <[email protected]>
All of our other file format schemas are in pkg/schema. This moves the file to the right location and adds a copy instruction in our build target to copy the template from the schema directory when we build. Make the schema available at /schema/v1/NAME.schema.json Signed-off-by: Carolyn Van Slyck <[email protected]>
0c74d27
to
95c97f4
Compare
Signed-off-by: Carolyn Van Slyck <[email protected]>
/azp run porter-integration |
Azure Pipelines successfully started running 1 pipeline(s). |
I'm avoiding returning a hard error immediately because then people would upgrade and no existing bundles would work for them. Maybe in the future we can change this to a hard check when more people have had a chance to migrate. Signed-off-by: Carolyn Van Slyck <[email protected]>
/azp run porter-integration |
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Carolyn Van Slyck <[email protected]>
| schemaType | false | The type of document. This isn't used by Porter but is included when Porter outputs the file, so that editors can determine the resource type. | | ||
| schemaVersion | true | The version of the Bundle schema used in this file. | | ||
| name | true | The name of the bundle. | | ||
| version | false | The version of the bundle, must adher to [semver v2]. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| version | false | The version of the bundle, must adher to [semver v2]. | | |
| version | false | The version of the bundle, must adhere to [semver v2]. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention what value will be used for the bundle's tag if version isn't provided? Just tested and it appears to be a SHA of some sort (which isn't in itself semver, right?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the docs to indicate that version is required, though people can set it programmatically during build thanks to the feature you added a while back. So it's not possible to avoid setting version.
The bundle still uses a tag defaulted using the version unless you set it with the reference field, or --tag when publishing.
What you are probably noticing is Yingrong's recent fix for how we first push the invocation image (which used to be named BUNDLENAME-installer), retrieved its repository digest and then patched the bundle.json before publishing the bundle. We now push the invocation image to a tag (a hash) in the same repository as the bundle. We still do the same trick to update the repository digest in the bundle before publishing. The difference is that we don't create additional repositories that are unused (i.e. the -installer image isn't actually used by the bundle, we use the one in BUNDLENAME@sha256:abc123). This makes it more clear to people that we are generating stuff that they shouldn't try to reference by name and that the image is distributed with the bundle.
Untagging so that we don't leave stuff around to confuse people isn't supported yet but is on the backlog.
Looks great to me! |
That's a great idea. I'll add someone about breaking changes to CONTRIBUTING.MD, as I think it's a general topic. Then also add some comments inline so people know "If you change this data structure, here's what you have to do next..." |
Signed-off-by: Carolyn Van Slyck <[email protected]>
@vdice Updated with everyone's review feedback. Please take another look when you can! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
CONTRIBUTING.md
Outdated
## Breaking Changes | ||
|
||
Some changes in Porter break our compatibility with previous versions of Porter. | ||
When that happens, we need to release that change a new major version number to indicate to users that it contains breaking changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When that happens, we need to release that change a new major version number to indicate to users that it contains breaking changes. | |
When that happens, we need to release that change with a new major version number to indicate to users that it contains breaking changes. |
Signed-off-by: Carolyn Van Slyck <[email protected]>
What does this change
This adds a document schema version to the Porter manifest (porter.yaml) similar to what we already have on credential and parameter sets. The schema of the any document should not change within a major version.
I've added a page that lists which schema versions are used by various porter versions.
Preview of doc changes at https://deploy-preview-1946--porter.netlify.app/reference/file-formats/
What issue does it fix
Closes #1566
Notes for the reviewer
I've created #1956 as a follow-up so that we start using the schemaVersion set.
Checklist
Reviewer Checklist