-
Notifications
You must be signed in to change notification settings - Fork 123
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
refactor: expose Kubeconform as a module #189
base: master
Are you sure you want to change the base?
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 |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package main | ||
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. I kinda liked having the main function in cmd/kubeconform... I might move the validation to pkg/kubeconform.. still thinking about this. 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. I saw different ways to make it, but if that's your preference, maybe putting the main back in 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. Another idea, we can just put 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. I've not completely made up my mind on this, I'm likely to try out something that would allow you to work, but I might reserve myself the right to break that location/interface for a little while while I find a good place for it :) |
||
|
||
import ( | ||
"fmt" | ||
"os" | ||
|
||
"github.com/yannh/kubeconform/cmd/kubeconform" | ||
"github.com/yannh/kubeconform/pkg/config" | ||
) | ||
|
||
var version = "development" | ||
|
||
func main() { | ||
cfg, out, err := config.FromFlags(os.Args[0], os.Args[1:]) | ||
if err != nil { | ||
fmt.Fprintf(os.Stderr, "failed parsing command line: %s\n", err.Error()) | ||
os.Exit(1) | ||
} | ||
|
||
if err = kubeconform.Validate(cfg, out); err != nil { | ||
fmt.Fprintf(os.Stderr, "failed validating resources: %s - %s\n", err.Error(), out) | ||
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. this is incorrect.. kubeconform currently returns 1 when some resources are not valid (but it didn't fail validating resources) - we need to differentiate errors processing from errors when validating.. 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. I didn't change the logic here, I just used what's there but with an error instead of an init. |
||
os.Exit(1) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,28 +4,58 @@ import ( | |
"bytes" | ||
"flag" | ||
"fmt" | ||
"io" | ||
"os" | ||
"strings" | ||
) | ||
|
||
type Stream struct { | ||
Input io.Reader | ||
Output io.Writer | ||
Error io.Writer | ||
} | ||
|
||
type Config struct { | ||
Cache string | ||
Debug bool | ||
ExitOnError bool | ||
Files []string | ||
SchemaLocations []string | ||
SkipTLS bool | ||
SkipKinds map[string]struct{} | ||
Cache string `yaml:"cache" json:"cache"` | ||
Debug bool `yaml:"debug" json:"debug"` | ||
ExitOnError bool `yaml:"exitOnError" json:"exitOnError"` | ||
Files []string `yaml:"files" json:"files"` | ||
Help bool `yaml:"help" json:"help"` | ||
IgnoreFilenamePatterns []string `yaml:"ignoreFilenamePatterns" json:"ignoreFilenamePatterns"` | ||
IgnoreMissingSchemas bool `yaml:"ignoreMissingSchemas" json:"ignoreMissingSchemas"` | ||
KubernetesVersion string `yaml:"kubernetesVersion" json:"kubernetesVersion"` | ||
NumberOfWorkers int `yaml:"numberOfWorkers" json:"numberOfWorkers"` | ||
OutputFormat string `yaml:"output" json:"output"` | ||
RejectKinds map[string]struct{} | ||
OutputFormat string | ||
KubernetesVersion string | ||
NumberOfWorkers int | ||
Summary bool | ||
Strict bool | ||
Verbose bool | ||
IgnoreMissingSchemas bool | ||
IgnoreFilenamePatterns []string | ||
Help bool | ||
Version bool | ||
RejectKindsNG []string `yaml:"reject" json:"reject"` | ||
SchemaLocations []string `yaml:"schemaLocations" json:"schemaLocations"` | ||
SkipKinds map[string]struct{} | ||
SkipKindsNG []string `yaml:"skip" json:"skip"` | ||
SkipTLS bool `yaml:"insecureSkipTLSVerify" json:"insecureSkipTLSVerify"` | ||
Strict bool `yaml:"strict" json:"strict"` | ||
Summary bool `yaml:"summary" json:"summary"` | ||
Verbose bool `yaml:"verbose" json:"verbose"` | ||
Version bool `yaml:"version" json:"version"` | ||
Stream *Stream | ||
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. I'm wondering if Stream fits better here, or as a separate argument to Validate(). 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. Maybe in I will think of a struct and it has something like:
which represents |
||
} | ||
|
||
// LoadNGConfig allows to introduce new config format/structure while keeping backward compatibility. | ||
func (c *Config) LoadNGConfig() error { | ||
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. I would suggest leaving this out for now, to make the PR easier to merge. |
||
// SkipKindsNG. | ||
loadKinds(c.SkipKinds, c.SkipKindsNG) | ||
|
||
// RejectKindsNG. | ||
loadKinds(c.RejectKinds, c.RejectKindsNG) | ||
|
||
return nil | ||
} | ||
|
||
func loadKinds(dest map[string]struct{}, src []string) { | ||
for _, kind := range src { | ||
if len(kind) > 0 { | ||
dest[kind] = struct{}{} | ||
} | ||
} | ||
} | ||
|
||
type arrayParam []string | ||
|
@@ -40,15 +70,8 @@ func (ap *arrayParam) Set(value string) error { | |
} | ||
|
||
func splitCSV(csvStr string) map[string]struct{} { | ||
splitValues := strings.Split(csvStr, ",") | ||
valuesMap := map[string]struct{}{} | ||
|
||
for _, kind := range splitValues { | ||
if len(kind) > 0 { | ||
valuesMap[kind] = struct{}{} | ||
} | ||
} | ||
|
||
loadKinds(valuesMap, strings.Split(csvStr, ",")) | ||
return valuesMap | ||
} | ||
|
||
|
@@ -62,7 +85,9 @@ func FromFlags(progName string, args []string) (Config, string, error) { | |
|
||
c := Config{} | ||
c.Files = []string{} | ||
c.Stream = &Stream{os.Stdin, os.Stdout, os.Stderr} | ||
|
||
flags.SetOutput(c.Stream.Output) | ||
flags.StringVar(&c.KubernetesVersion, "kubernetes-version", "master", "version of Kubernetes to validate against, e.g.: 1.18.0") | ||
flags.Var(&schemaLocationsParam, "schema-location", "override schemas location search path (can be specified multiple times)") | ||
flags.StringVar(&skipKindsCSV, "skip", "", "comma-separated list of kinds or GVKs to ignore") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ package output | |
|
||
import ( | ||
"fmt" | ||
"os" | ||
"io" | ||
|
||
"github.com/yannh/kubeconform/pkg/validator" | ||
) | ||
|
@@ -12,9 +12,7 @@ type Output interface { | |
Flush() error | ||
} | ||
|
||
func New(outputFormat string, printSummary, isStdin, verbose bool) (Output, error) { | ||
w := os.Stdout | ||
|
||
func New(w io.Writer, outputFormat string, printSummary, isStdin, verbose bool) (Output, error) { | ||
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. I quite like the ability to override the streams here 👍 |
||
switch { | ||
case outputFormat == "json": | ||
return jsonOutput(w, printSummary, isStdin, verbose), 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 still refer to os.Stdin here instead of cfg.stream.stdin, but here we need Stat()
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.
yeah, I put it away to think about the best way to make it.
I will get back to it.
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.
Hold on a bit - I'm not super happy with the signature of "Validate" yet, I would want it to enable the caller to process the results in a way they see fit. I need to think about this a bit more 🙇
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.
great 👌 👌