Build improvements (#146)

* Fix usage of ** in glob

  - Add new dependency becuase golang filepath.glob doesn't support
    recursive matching.

* Allow output directory to be configured

  - This fixes #144.
  - Improved configuration yaml file property casing.

* Add makefile target for running k8sinfra-gen

* Little comment type fixup

* Allow `make gen-arm` to be rerun by deleting the generated code

Since the generated code doesn't currently build it would prevent
`make gen-arm` from being rerun until the apis directory was
removed.

Hit a bug in codegen.isFileGenerated that would fail to close
files - move the deferred close to before the early return.

* Add clarifying comment to Makefile

Co-authored-by: Christian Muirhead <christian.muirhead@canonical.com>

hack/generator/.gitignore

@@ -1,2 +1,2 @@
 apis/
-config/crd/bases/
+config/crd/bases/

hack/generator/Makefile

@@ -17,6 +17,17 @@ all: build test
 manifests: $(CONTROLLER_GEN)
 	$(Q) $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./apis/..." output:crd:artifacts:config=config/crd/bases
 
+.PHONY: gen-arm
+gen-arm: rm-apis build ; $(info $(M) running k8sinfra-gen gen…) @ ## Running gen
+	$(Q) ./bin/$(APP) gen azure-arm.yaml
+
+# TODO: We should remove this target once we place the generated code outside of the generator.
+# TODO: The generator already cleans up the APIs directory but since right now the generated code
+# TODO: doesn't build we need to rm-apis before build.
+.PHONY: rm-apis
+rm-apis: ; $(info $(M) removing previously generated apis…)
+	$(Q) rm -rf apis
+
 .PHONY: build
 build: tidy fmt vet lint ; $(info $(M) buiding ./bin/$(APP))
 	$(Q) $(GO)  build -ldflags "-X $(PACKAGE)/cmd.GitCommit=$(VERSION)" -o ./bin/$(APP)
@@ -65,4 +76,4 @@ gox: install-tools
 	$(Q) tar -czvf ./bin/$(SHORT_VERSION)/$(APP)_windows_amd64.tar.gz -C ./bin/$(SHORT_VERSION)/ $(APP)_windows_amd64.exe
 
 .PHONY: ci
-ci: build test-cover
+ci: build test-cover

hack/generator/azure-cloud.yaml → hack/generator/azure-arm.yaml

@@ -1,10 +1,11 @@
-schemaurl: https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json
-typefilters:
+schemaUrl: https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json
+outputPath: apis
+typeFilters:
   - action: prune
     group: definitions
     name: Expression
     because: expression is an ARM template construct which doesn't belong in CRDs
-exportfilters:
+exportFilters:
   - action: include
     version: 2020-*
     because: all 2020 API versions are included
@@ -22,7 +23,7 @@ exportfilters:
     group: definitions
     name: ResourceBase
     because: this type is not used
-typetransformers:
+typeTransformers:
   - group: definitions
     name: NumberOrExpression
     because: NumberOrExpression is an ARM template artifact that doesn't make sense in CRDs

hack/generator/azure-stack.yaml

@@ -1,5 +1,5 @@
-schemaurl: https://schema.management.azure.com/schemas/2019-03-01-hybrid/deploymentTemplate.json
-exportfilters:
+schemaUrl: https://schema.management.azure.com/schemas/2019-03-01-hybrid/deploymentTemplate.json
+exportFilters:
   - action: include
     version: 2020-*
     because: all 2020 API versions are included

hack/generator/cmd/gen/gen.go

@@ -18,17 +18,22 @@ import (
 // NewGenCommand creates a new cobra Command when invoked from the command line
 func NewGenCommand() (*cobra.Command, error) {
 	cmd := &cobra.Command{
-		Use:   "gen",
+		// TODO: there's not great support for required
+		// TODO: arguments in cobra so this is the best we get... see:
+		// TODO: https://github.com/spf13/cobra/issues/395
+		Use:   "gen <config>",
 		Short: "generate K8s infrastructure resources from Azure deployment template schema",
+		Args:  cobra.ExactArgs(1),
 		Run: xcobra.RunWithCtx(func(ctx context.Context, cmd *cobra.Command, args []string) error {
+			configFile := args[0]
 
-			cg, err := codegen.NewCodeGenerator("azure-cloud.yaml")
+			cg, err := codegen.NewCodeGenerator(configFile)
 			if err != nil {
 				klog.Errorf("Error creating code generator: %v\n", err)
 				return err
 			}
 
-			err = cg.Generate(ctx, "apis")
+			err = cg.Generate(ctx)
 			if err != nil {
 				klog.Errorf("Error during code generation:\n%v\n", err)
 				return err

hack/generator/cmd/root.go

@@ -6,9 +6,7 @@
 package cmd
 
 import (
-	"fmt"
 	"github.com/spf13/cobra"
-	"github.com/spf13/viper"
 	"k8s.io/klog/v2"
 
 	"github.com/Azure/k8s-infra/hack/generator/cmd/gen"
@@ -35,12 +33,6 @@ func newRootCommand() (*cobra.Command, error) {
 
 	rootCmd.Flags().SortFlags = false
 
-	var cfgFile string
-	rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (defaults are ./config.yaml, $HOME/.k8sinfra/config.yaml, /etc/k8sinfra/config.yaml)")
-	cobra.OnInitialize(func() {
-		initConfig(&cfgFile)
-	})
-
 	cmdFuncs := []func() (*cobra.Command, error){
 		gen.NewGenCommand,
 	}
@@ -55,28 +47,3 @@ func newRootCommand() (*cobra.Command, error) {
 
 	return rootCmd, nil
 }
-
-func initConfig(cfgFilePtr *string) {
-	cfgFile := *cfgFilePtr
-	if cfgFile != "" {
-		// Use config file from the flag.
-		viper.SetConfigFile(cfgFile)
-	} else {
-		viper.SetConfigName("config")
-		viper.AddConfigPath("/etc/k8sinfra/")
-		viper.AddConfigPath("$HOME/.k8sinfra")
-		viper.AddConfigPath(".")
-	}
-
-	viper.AutomaticEnv()
-	if err := viper.ReadInConfig(); err != nil {
-		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
-			// Config file not found; set sane defaults and carry on.
-			fmt.Println("Configuration file not found.")
-		} else {
-			// Config file was found but another error was produced
-			panic(fmt.Errorf("Fatal error reading config file: %s", err))
-		}
-	}
-	fmt.Println("Found configuration file.")
-}

hack/generator/go.mod

@@ -3,6 +3,7 @@ module github.com/Azure/k8s-infra/hack/generator
 go 1.13
 
 require (
+	github.com/bmatcuk/doublestar v1.3.1
 	github.com/devigned/tab v0.1.1
 	github.com/onsi/gomega v1.8.1
 	github.com/pelletier/go-toml v1.6.0 // indirect

hack/generator/go.sum

@@ -12,6 +12,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
+github.com/bmatcuk/doublestar v1.3.1 h1:rT8rxDPsavp9G+4ZULzqhhUSaI/OPsTZNG88Z3i0xvY=
+github.com/bmatcuk/doublestar v1.3.1/go.mod h1:wiQtGV+rzVYxB7WIlirSN++5HPtPlXEo9MEoZQC/PmE=
 github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=

hack/generator/pkg/codegen/code_generator.go

@@ -22,6 +22,7 @@ import (
 	"github.com/Azure/k8s-infra/hack/generator/pkg/config"
 	"github.com/Azure/k8s-infra/hack/generator/pkg/jsonast"
 	"github.com/xeipuuv/gojsonreference"
+	"github.com/bmatcuk/doublestar"
 	"github.com/xeipuuv/gojsonschema"
 	"gopkg.in/yaml.v3"
 
@@ -52,17 +53,17 @@ func NewCodeGenerator(configurationFile string) (*CodeGenerator, error) {
 }
 
 // Generate produces the Go code corresponding to the configured JSON schema in the given output folder
-func (generator *CodeGenerator) Generate(ctx context.Context, outputFolder string) error {
+func (generator *CodeGenerator) Generate(ctx context.Context) error {
 	klog.V(0).Infof("Loading JSON schema %v", generator.configuration.SchemaURL)
 	schema, err := loadSchema(ctx, generator.configuration.SchemaURL)
 	if err != nil {
 		return fmt.Errorf("error loading schema from '%v' (%w)", generator.configuration.SchemaURL, err)
 	}
 
-	klog.V(0).Infof("Cleaning output folder '%v'", outputFolder)
-	err = deleteGeneratedCodeFromFolder(ctx, outputFolder)
+	klog.V(0).Infof("Cleaning output folder '%v'", generator.configuration.OutputPath)
+	err = deleteGeneratedCodeFromFolder(ctx, generator.configuration.OutputPath)
 	if err != nil {
-		return fmt.Errorf("error cleaning output folder '%v' (%w)", outputFolder, err)
+		return fmt.Errorf("error cleaning output folder '%v' (%w)", generator.configuration.OutputPath, err)
 	}
 
 	scanner := jsonast.NewSchemaScanner(astmodel.NewIdentifierFactory(), generator.configuration)
@@ -93,14 +94,14 @@ func (generator *CodeGenerator) Generate(ctx context.Context, outputFolder strin
 	definitionCount := 0
 
 	// emit each package
-	klog.V(0).Infof("Writing output files into %v", outputFolder)
+	klog.V(0).Infof("Writing output files into %v", generator.configuration.OutputPath)
 	for _, pkg := range packages {
 		if ctx.Err() != nil { // check for cancellation
 			return ctx.Err()
 		}
 
 		// create directory if not already there
-		outputDir := filepath.Join(outputFolder, pkg.GroupName, pkg.PackageName)
+		outputDir := filepath.Join(generator.configuration.OutputPath, pkg.GroupName, pkg.PackageName)
 		if _, err := os.Stat(outputDir); os.IsNotExist(err) {
 			klog.V(5).Infof("Creating directory '%s'\n", outputDir)
 			err = os.MkdirAll(outputDir, 0700)
@@ -359,9 +360,10 @@ func loadSchema(ctx context.Context, source string) (*gojsonschema.Schema, error
 }
 
 func deleteGeneratedCodeFromFolder(ctx context.Context, outputFolder string) error {
-	globPattern := path.Join(outputFolder, "**", "*", "*"+astmodel.CodeGeneratedFileSuffix) + "*"
+	// We use doublestar here rather than filepath.Glob because filepath.Glob doesn't support **
+	globPattern := path.Join(outputFolder, "**", "*", "*"+astmodel.CodeGeneratedFileSuffix)
 
-	files, err := filepath.Glob(globPattern)
+	files, err := doublestar.Glob(globPattern)
 	if err != nil {
 		return fmt.Errorf("error globbing files with pattern '%s' (%w)", globPattern, err)
 	}
@@ -405,6 +407,7 @@ func isFileGenerated(filename string) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	defer f.Close()
 
 	reader := bufio.NewReader(f)
 	for i := 0; i < maxLinesToCheck; i++ {
@@ -420,7 +423,6 @@ func isFileGenerated(filename string) (bool, error) {
 			return true, nil
 		}
 	}
-	defer f.Close()
 
 	return false, nil
 }

hack/generator/pkg/config/configuration.go

@@ -16,13 +16,15 @@ import (
 // Configuration is used to control which types get generated
 type Configuration struct {
 	// Base URL for the JSON schema to generate
-	SchemaURL string
+	SchemaURL string `yaml:"schemaUrl"`
+	// The folder where the code should be generated
+	OutputPath string `yaml:"outputPath"`
 	// Filters used to control which types are exported
-	ExportFilters []*ExportFilter
+	ExportFilters []*ExportFilter `yaml:"exportFilters"`
 	// Filters used to control which types are created from the JSON schema
-	TypeFilters []*TypeFilter
+	TypeFilters []*TypeFilter `yaml:"typeFilters"`
 	// Transformers used to remap types
-	TypeTransformers []*TypeTransformer
+	TypeTransformers []*TypeTransformer `yaml:"typeTransformers"`
 }
 
 // ShouldExportResult is returned by ShouldExport to indicate whether the supplied type should be exported
@@ -66,6 +68,11 @@ func (config *Configuration) Initialize() error {
 		return errors.New("SchemaURL missing")
 	}
 
+	if config.OutputPath == "" {
+		// Default to an apis folder in the current directory if not specified
+		config.OutputPath = "apis"
+	}
+
 	var errs []error
 	for _, filter := range config.ExportFilters {
 		err := filter.Initialize()

pkg/util/patch/patch.go

@@ -106,7 +106,7 @@ func (h *Helper) Patch(ctx context.Context, resource runtime.Object) error {
 	if ok {
 		hasStatus = true
 		// if the resource contains a status remove it from our unstructured copy
-		// to avoid uneccsary patching.
+		// to avoid unnecessary patching.
 		unstructured.RemoveNestedField(after, "status")
 	}