fix: catch errors with defaultNamespace in params.yaml #86
Conversation
…en running build or apply Also moved loading params.yaml to the top of GenerateKustomizeResult so that we can do error checking before any file copying - which is slow.
| } | ||
|
|
||
| // HumanizeKustomizeError takes errors returned from GenerateKustomizeResult and returns them in a human friendly string | ||
| func HumanizeKustomizeError(err error) string { |
There was a problem hiding this comment.
Should this be HumanizeKustomizeNamespaceError since the error messages are namespace specific?
There was a problem hiding this comment.
We could do that. I figured it could expand to other errors. This is specific to the GenerateKustomizeResult function. So maybe we should call it HumanizeKustomizeResultError?
| switch paramsError.ErrorType { | ||
| case "missing": | ||
| return fmt.Sprintf("%s is missing in your params.yaml", paramsError.Key) | ||
| case "blank": |
There was a problem hiding this comment.
Or maybe these ErrorTypes should be namespaceBlank and namespaceReserved?
There was a problem hiding this comment.
@rushtehrani That might be best. Although, it is possible other params could be missing or blank when they should not be - the messages would be the same.
|
|
||
| defaultNamespace := manifest.GetValue("application.defaultNamespace") | ||
| if defaultNamespace == nil { | ||
| return &ParamsError{Key: "application.defaultNamespace", ErrorType: "missing"} |
There was a problem hiding this comment.
@rushtehrani That might be best. Although, it is possible other params could be missing or blank when they should not be - the messages would be the same.
Would it make sense to then have ErrorType and ResourceName and then provide a message based on the combination of those? And maybe have a catch all condition for each ErrorType if the ResourceName is not defined. We could also create a separate issue for this and expand later.
There was a problem hiding this comment.
Yeah, let's expand on this separately.
I'm completely up for expanding/fleshing this out. The important parts to me are:
- The key that caused the issue
- The value (if applicable) that caused the issue.
- What type of error it is - value missing, value blank, value invalid otherwise (how? reserved name, too many characters, etc)
- Bonus: Line that caused the issue.
What this PR does:
Catches errors with params.yaml when the default namespace is blank, missing, or a reserved namespace.
Which issue(s) this PR fixes:
Fixes onepanelio/onepanel#748
Special notes for your reviewer: