Added validation if someone specifies a version on a bundle #3148
Conversation
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
dgannon991
force-pushed
the
feat/3136/mixin-version-support
branch
from
2a2d8e8 to
4ebfb8e
Compare
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
dgannon991
requested review from
schristoff,
sgettys,
bdegeeter,
troy0820 and
kichristensen
as code owners
|
I've added some tests and done a small refactor on the bottom of Lint(). Let me know if we want to break further parts of the function out! |
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
| func (l *Linter) validateVersionNumberConstraints(ctx context.Context, m *manifest.Manifest) error { | ||
| for _, mixin := range m.Mixins { | ||
| if mapConfig, ok := mixin.Config.(map[string]interface{}); ok { | ||
| if v, exists := mapConfig["version"]; exists { |
There was a problem hiding this comment.
Are there currently anything ensuring that mixins cannot use a config field called version?
I'm thinking of the situation where mixins already provide a field called version, which might result in unintended behaviour
There was a problem hiding this comment.
No, it was simply the suggested field name on the issue. We could always use mixinVersion or something more verbose to avoid clashing?
There was a problem hiding this comment.
I personally like version the best, but it could break some existing plugins.
@schristoff do you know of any way we can easily handle a "reservation" of a mixin configuration property?
There was a problem hiding this comment.
@kichristensen, @schristoff could a similar approach such as porter-state, porter-debug be taken?
|
I think we can close this one, it was done in a different way. |
|
Closed in favor of #3215 |
What does this change
Added a check on version numbers if specified in your porter YAML
What issue does it fix
Delivers towards #3136
Notes for the reviewer
This is just a draft to get some feedback. I intend to refactor the Lint() function and add some unit tests, but I wanted to check if I'd gone down the right route first :)