Release input package spec as GA #485
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
|
|
||
| func getKibanaVersionCondition(manifest pkgpath.File) (string, error) { | ||
|
|
||
| val, err := manifest.Values("$.conditions[\"kibana.version\"]") |
There was a problem hiding this comment.
do we need to handle:
conditions:
kibana.version: 1234
and
conditions:
kibana:
version: 1234
here? It seems the lib treats them differently, all packages use kibana.version
There was a problem hiding this comment.
This actually looks like an issue with this Values helper. The spec allows both kibana.version, or version as an attribute inside kibana.
It should probably use ucfg to parse these files instead of plain yaml. See https://github.com/elastic/go-ucfg#dot-notations
There was a problem hiding this comment.
I'll have a go at this 👌
There was a problem hiding this comment.
Both ways of defining Kibana version condition are used in the integrations repository. For instance:
There was a problem hiding this comment.
I have put a temporary workaround in this PR (9840df4)and have a draft PR for using ucfg https://github.com/elastic/package-spec/pull/487/files
🌐 Coverage report
|
| manifest, err := readManifest(fsys) | ||
| if err != nil { | ||
| return ve.ValidationErrors{err} | ||
| } | ||
|
|
||
| packageType, err := getPackageType(*manifest) | ||
| if err != nil { | ||
| return ve.ValidationErrors{err} | ||
| } | ||
|
|
||
| packageVersion, err := getPackageVersion(*manifest) | ||
| if err != nil { | ||
| return ve.ValidationErrors{err} | ||
| } |
There was a problem hiding this comment.
Probably, these fields can be retrieved from the Package struct that it is returned in NewPackageFromFS function https://github.com/elastic/package-spec/blob/main/code/go/internal/validator/package.go#L55
But Kibana version condition is not available from there.
There was a problem hiding this comment.
Ah I missed this util, thanks!
There was a problem hiding this comment.
Cyclic dependency prevents me from doing this unfortunately
There was a problem hiding this comment.
An option to break this cyclic dependency could be move all the code/go/internal/validator/package*.go files to its own package. For instance it could be created a new package like code/go/internal/packages/ and update all the references.
|
|
||
| func getKibanaVersionCondition(manifest pkgpath.File) (string, error) { | ||
|
|
||
| val, err := manifest.Values("$.conditions[\"kibana.version\"]") |
There was a problem hiding this comment.
Both ways of defining Kibana version condition are used in the integrations repository. For instance:
| manifest, err := readManifest(fsys) | ||
| if err != nil { | ||
| return ve.ValidationErrors{err} | ||
| } | ||
|
|
||
| packageType, err := getPackageType(*manifest) | ||
| if err != nil { | ||
| return ve.ValidationErrors{err} | ||
| } | ||
|
|
||
| packageVersion, err := getPackageVersion(*manifest) | ||
| if err != nil { | ||
| return ve.ValidationErrors{err} | ||
| } |
There was a problem hiding this comment.
An option to break this cyclic dependency could be move all the code/go/internal/validator/package*.go files to its own package. For instance it could be created a new package like code/go/internal/packages/ and update all the references.
| sVersion, err := semver.NewVersion(sVal) | ||
| return sVersion, nil |
There was a problem hiding this comment.
you can return directly the value from semver.NewVersion function
| sVersion, err := semver.NewVersion(sVal) | |
| return sVersion, nil | |
| return semver.NewVersion(sVal) |
|
@mrodm I have resolved the cyclic dependency issue, my validator now uses |
What does this PR do?
Removes
release: betafrom the input package spec.Adds validation to check that a GA input package enforces kibana version must be ^8.8.0.
Changes the test input packages to test this new behaviour.
Why is it important?
This allows input packages to have release versions >= 1.0.0. For example the custom logs integration
Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.Related issues
Closes #480