-
Notifications
You must be signed in to change notification settings - Fork 243
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 container name validation during creation and push #3959
Changes from 4 commits
a9932b7
8870d8a
40011ab
98420af
aafdfaa
ac976a8
f92f9cb
60ea264
f68b19e
2850963
1327131
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import ( | |
ktemplates "k8s.io/kubectl/pkg/util/templates" | ||
|
||
"github.com/openshift/odo/pkg/component" | ||
adaptersCommon "github.com/openshift/odo/pkg/devfile/adapters/common" | ||
"github.com/openshift/odo/pkg/devfile/parser" | ||
"github.com/openshift/odo/pkg/log" | ||
projectCmd "github.com/openshift/odo/pkg/odo/cli/project" | ||
|
@@ -175,6 +176,28 @@ func (po *PushOptions) Validate() (err error) { | |
// If Devfile is present we do not need to validate the below S2I checks | ||
// TODO: Perhaps one day move Devfile validation to here instead? | ||
if util.CheckPathExists(po.DevfilePath) { | ||
spinner := log.Spinner("Validating devfile component") | ||
defer spinner.End(false) | ||
err = util.ValidateK8sResourceName("component name", po.EnvSpecificInfo.GetName()) | ||
if err != nil { | ||
return err | ||
} | ||
containerComponents := adaptersCommon.GetDevfileContainerComponents(po.Devfile.Data) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can extract https://github.com/openshift/odo/pull/3959/files#diff-6e3a2602ffd394e9a73d4fbf1388be8dR185-R190 out into a helper function and use it in create as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated to use unit tests instead of integration tests |
||
for _, comp := range containerComponents { | ||
err := util.ValidateK8sResourceName("container name", comp.Name) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
// Only validate namespace if pushtarget isn't docker | ||
if !pushtarget.IsPushTargetDocker() { | ||
err := util.ValidateK8sResourceName("component namespace", po.EnvSpecificInfo.GetNamespace()) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
spinner.End(true) | ||
return nil | ||
} | ||
|
||
|
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.
we should validate under one spinner
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.
updated