-
Notifications
You must be signed in to change notification settings - Fork 32
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
🐛 Validate source and target labels #243
Changes from all commits
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 |
---|---|---|
|
@@ -2,6 +2,7 @@ package cmd | |
|
||
import ( | ||
"bufio" | ||
"bytes" | ||
"context" | ||
"encoding/json" | ||
"errors" | ||
|
@@ -113,7 +114,7 @@ func NewAnalyzeCmd(log logr.Logger) *cobra.Command { | |
return err | ||
} | ||
} | ||
err := analyzeCmd.Validate() | ||
err := analyzeCmd.Validate(cmd.Context()) | ||
if err != nil { | ||
log.Error(err, "failed to validate flags") | ||
return err | ||
|
@@ -225,13 +226,48 @@ func NewAnalyzeCmd(log logr.Logger) *cobra.Command { | |
return analyzeCommand | ||
} | ||
|
||
func (a *analyzeCommand) Validate() error { | ||
func (a *analyzeCommand) Validate(ctx context.Context) error { | ||
if a.listSources || a.listTargets { | ||
return nil | ||
} | ||
if a.labelSelector != "" && (len(a.sources) > 0 || len(a.targets) > 0) { | ||
return fmt.Errorf("must not specify label-selector and sources or targets") | ||
} | ||
// Validate source labels | ||
if len(a.sources) > 0 { | ||
var sourcesRaw bytes.Buffer | ||
a.fetchLabels(ctx, true, false, &sourcesRaw) | ||
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. Re-used ListLabels extracted method body to have the share the logic for fetching sources/targets from analyzer. However, this doesn't look as nice as I'd like, looking on update. 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. Is there some way to use the slice created here https://github.com/konveyor/kantra/blob/main/cmd/analyze.go#L454 to return that in validate, and then re-use it to list everything after? 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. @eemcmullan I guess it won't be possible because validation runs locally, list commands work in container |
||
knownSources := strings.Split(sourcesRaw.String(), "\n") | ||
for _, source := range a.sources { | ||
found := false | ||
for _, knownSource := range knownSources { | ||
if source == knownSource { | ||
found = true | ||
} | ||
} | ||
if !found { | ||
return fmt.Errorf("unknown source: \"%s\"", source) | ||
} | ||
} | ||
} | ||
// Validate source labels | ||
if len(a.targets) > 0 { | ||
var targetRaw bytes.Buffer | ||
a.fetchLabels(ctx, false, true, &targetRaw) | ||
knownTargets := strings.Split(targetRaw.String(), "\n") | ||
for _, source := range a.targets { | ||
found := false | ||
for _, knownTarget := range knownTargets { | ||
if source == knownTarget { | ||
found = true | ||
} | ||
} | ||
if !found { | ||
return fmt.Errorf("unknown target: \"%s\"", source) | ||
} | ||
} | ||
} | ||
|
||
// do not allow multiple input applications | ||
inputNum := 0 | ||
for _, arg := range os.Args { | ||
|
@@ -327,29 +363,33 @@ func (a *analyzeCommand) CheckOverwriteOutput() error { | |
return nil | ||
} | ||
|
||
func (a *analyzeCommand) ListLabels(ctx context.Context) error { | ||
func (a *analyzeCommand) ListLabels(ctx context.Context) error { | ||
return a.fetchLabels(ctx, a.listSources, a.listTargets, os.Stdout) | ||
} | ||
|
||
func (a *analyzeCommand) fetchLabels(ctx context.Context, listSources, listTargets bool, out io.Writer) error { | ||
// reserved labels | ||
sourceLabel := outputv1.SourceTechnologyLabel | ||
targetLabel := outputv1.TargetTechnologyLabel | ||
runMode := "RUN_MODE" | ||
runModeContainer := "container" | ||
if os.Getenv(runMode) == runModeContainer { | ||
if a.listSources { | ||
if listSources { | ||
sourceSlice, err := readRuleFilesForLabels(sourceLabel) | ||
if err != nil { | ||
a.log.Error(err, "failed to read rule labels") | ||
return err | ||
} | ||
listOptionsFromLabels(sourceSlice, sourceLabel) | ||
listOptionsFromLabels(sourceSlice, sourceLabel, out) | ||
return nil | ||
} | ||
if a.listTargets { | ||
if listTargets { | ||
targetsSlice, err := readRuleFilesForLabels(targetLabel) | ||
if err != nil { | ||
a.log.Error(err, "failed to read rule labels") | ||
return err | ||
} | ||
listOptionsFromLabels(targetsSlice, targetLabel) | ||
listOptionsFromLabels(targetsSlice, targetLabel, out) | ||
return nil | ||
} | ||
} else { | ||
|
@@ -359,7 +399,7 @@ func (a *analyzeCommand) ListLabels(ctx context.Context) error { | |
return err | ||
} | ||
args := []string{"analyze"} | ||
if a.listSources { | ||
if listSources { | ||
args = append(args, "--list-sources") | ||
} else { | ||
args = append(args, "--list-targets") | ||
|
@@ -373,6 +413,7 @@ func (a *analyzeCommand) ListLabels(ctx context.Context) error { | |
container.WithEntrypointBin(fmt.Sprintf("/usr/local/bin/%s", Settings.RootCommandName)), | ||
container.WithContainerToolBin(Settings.PodmanBinary), | ||
container.WithEntrypointArgs(args...), | ||
container.WithStdout(out), | ||
container.WithCleanup(a.cleanup), | ||
) | ||
if err != nil { | ||
|
@@ -434,7 +475,7 @@ func getSourceOrTargetLabel(text string, label string) string { | |
return "" | ||
} | ||
|
||
func listOptionsFromLabels(sl []string, label string) { | ||
func listOptionsFromLabels(sl []string, label string, out io.Writer) { | ||
var newSl []string | ||
l := label + "=" | ||
|
||
|
@@ -454,12 +495,12 @@ func listOptionsFromLabels(sl []string, label string) { | |
sort.Strings(newSl) | ||
|
||
if label == outputv1.SourceTechnologyLabel { | ||
fmt.Println("available source technologies:") | ||
fmt.Println(out, "available source technologies:") | ||
} else { | ||
fmt.Println("available target technologies:") | ||
fmt.Println(out, "available target technologies:") | ||
} | ||
for _, tech := range newSl { | ||
fmt.Println(tech) | ||
fmt.Println(out, tech) | ||
} | ||
} | ||
|
||
|
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.
Adding context to Validate() for non-trivial validations (running analyzer container for listing sources/targets in this case).