From 146e6834d4f83bf907290049a8bc7ff54a869f49 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 20 Apr 2023 15:07:06 +1200 Subject: [PATCH 01/21] Plumb new logging into gen-types --- v2/tools/generator/gen_types.go | 17 ++++++---- v2/tools/generator/root.go | 56 +++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/v2/tools/generator/gen_types.go b/v2/tools/generator/gen_types.go index ffdb2565e5f..e444e04fda5 100644 --- a/v2/tools/generator/gen_types.go +++ b/v2/tools/generator/gen_types.go @@ -12,7 +12,6 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/codegen" ) @@ -32,9 +31,11 @@ func NewGenTypesCommand() (*cobra.Command, error) { configFile := args[0] ctx := cmd.Context() + log := CreateLogger() + cg, err := codegen.NewCodeGeneratorFromConfigFile(configFile) if err != nil { - klog.Errorf("Error creating code generator: %s\n", err) + log.Error(err, "Error creating code generator") return err } @@ -42,19 +43,23 @@ func NewGenTypesCommand() (*cobra.Command, error) { var tmpDir string tmpDir, err = ioutil.TempDir("", createDebugPrefix(*debugMode)) if err != nil { - klog.Errorf("Error creating temporary directory: %s\n", err) + log.Error(err, "Error creating temporary directory") return err } - klog.V(0).Infof("Debug output will be written to the folder %s\n", tmpDir) + log.Info( + "Debug output will be written", + "folder", tmpDir) cg.UseDebugMode(*debugMode, tmpDir) defer func() { // Write the debug folder again so the user doesn't have to scroll back - klog.V(0).Infof("Debug output is available in folder %s\n", tmpDir) + log.Info( + "Debug output available", + "folder", tmpDir) }() } - err = cg.Generate(ctx) + err = cg.Generate(ctx, log) if err != nil { return logAndExtractStack("Error during code generation", err) diff --git a/v2/tools/generator/root.go b/v2/tools/generator/root.go index e96b40eb5e9..90c3881f6a3 100644 --- a/v2/tools/generator/root.go +++ b/v2/tools/generator/root.go @@ -7,23 +7,29 @@ package main import ( "context" + "os" "github.com/Azure/azure-service-operator/v2/internal/version" "github.com/Azure/azure-service-operator/v2/pkg/xcontext" + "github.com/go-logr/logr" + "github.com/go-logr/zerologr" + "github.com/rs/zerolog" "github.com/spf13/cobra" - "k8s.io/klog/v2" ) // Execute kicks off the command line func Execute() { cmd, err := newRootCommand() if err != nil { - klog.Fatalf("fatal error: commands failed to build! %s\n", err) + log := CreateLogger() + log.Error(err, "failed to create root command") + return } ctx := xcontext.MakeInterruptibleContext(context.Background()) if err := cmd.ExecuteContext(ctx); err != nil { - klog.Fatalln(err) + log := CreateLogger() + log.Error(err, "failed to execute root command") } } @@ -32,10 +38,15 @@ func newRootCommand() (*cobra.Command, error) { Use: "aso-gen", Short: "aso-gen provides a cmdline interface for generating Azure Service Operator types from Azure deployment template schema", TraverseChildren: true, + SilenceErrors: true, // We show errors ourselves using our logger + SilenceUsage: true, // Let users ask for usage themselves } rootCmd.Flags().SortFlags = false + rootCmd.PersistentFlags().BoolVar(&verbose, "verbose", false, "Enable verbose logging") + rootCmd.PersistentFlags().BoolVar(&quiet, "quiet", false, "Suppress non-error logging") + cmdFuncs := []func() (*cobra.Command, error){ NewGenTypesCommand, NewGenKustomizeCommand, @@ -50,5 +61,44 @@ func newRootCommand() (*cobra.Command, error) { rootCmd.AddCommand(cmd) } + rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + // Configure logging; --verbose overrides --quiet + if verbose { + zerologr.SetMaxV(1) + } else if quiet { + // Can't use zerologr.SetMaxV(-1) + zerolog.SetGlobalLevel(zerolog.ErrorLevel) + } else { + zerologr.SetMaxV(0) + } + } + return rootCmd, nil } + +var ( + verbose bool + quiet bool +) + +// CreateLogger creates a logger for console output. +// Use this when your command wants to show only log messages +func CreateLogger() logr.Logger { + + // Configure console writer for ZeroLog + output := zerolog.ConsoleWriter{ + Out: os.Stderr, // Write to StdErr + TimeFormat: "15:04:05.999", // Display time to the millisecond + } + + // Create zerolog logger + zl := zerolog.New(output). + With().Timestamp(). + Logger() + + // Use standard interface for logging + zerologr.VerbosityFieldName = "" // Don't include verbosity in output + + log := zerologr.New(&zl) + return log +} From f7dd2a46dbbd5e73fec0abf0ac0845da2f01bf5d Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 20 Apr 2023 15:07:31 +1200 Subject: [PATCH 02/21] Make log available to pipeline stages --- v2/tools/generator/gen_types.go | 2 +- .../internal/codegen/code_generator.go | 95 ++++++++++++------- 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/v2/tools/generator/gen_types.go b/v2/tools/generator/gen_types.go index e444e04fda5..7e13874abfc 100644 --- a/v2/tools/generator/gen_types.go +++ b/v2/tools/generator/gen_types.go @@ -33,7 +33,7 @@ func NewGenTypesCommand() (*cobra.Command, error) { log := CreateLogger() - cg, err := codegen.NewCodeGeneratorFromConfigFile(configFile) + cg, err := codegen.NewCodeGeneratorFromConfigFile(configFile, log) if err != nil { log.Error(err, "Error creating code generator") return err diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index c6db7edea0b..8ec863a4688 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -10,10 +10,9 @@ import ( "fmt" "time" - "github.com/pkg/errors" - "k8s.io/klog/v2" - "github.com/Azure/azure-service-operator/v2/internal/version" + "github.com/go-logr/logr" + "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/codegen/pipeline" @@ -28,7 +27,12 @@ type CodeGenerator struct { } // NewCodeGeneratorFromConfigFile produces a new Generator with the given configuration file -func NewCodeGeneratorFromConfigFile(configurationFile string) (*CodeGenerator, error) { +// configurationFile is the path to the configuration file. +// log is captured by stages to log messages. +func NewCodeGeneratorFromConfigFile( + configurationFile string, + log logr.Logger, +) (*CodeGenerator, error) { configuration, err := config.LoadConfiguration(configurationFile) if err != nil { return nil, err @@ -39,17 +43,22 @@ func NewCodeGeneratorFromConfigFile(configurationFile string) (*CodeGenerator, e return nil, err } - return NewTargetedCodeGeneratorFromConfig(configuration, astmodel.NewIdentifierFactory(), target) + return NewTargetedCodeGeneratorFromConfig(configuration, astmodel.NewIdentifierFactory(), target, log) } // NewTargetedCodeGeneratorFromConfig produces a new code generator with the given configuration and // only the stages appropriate for the specified target. +// configuration is used to configure the pipeline. +// idFactory is used to create identifiers for types. +// target is the target for which code is being generated. +// log is captured by stages to log messages. func NewTargetedCodeGeneratorFromConfig( configuration *config.Configuration, idFactory astmodel.IdentifierFactory, target pipeline.Target, + log logr.Logger, ) (*CodeGenerator, error) { - result, err := NewCodeGeneratorFromConfig(configuration, idFactory) + result, err := NewCodeGeneratorFromConfig(configuration, idFactory, log) if err != nil { return nil, errors.Wrapf(err, "creating pipeline targeting %s", target) } @@ -68,19 +77,31 @@ func NewTargetedCodeGeneratorFromConfig( } // NewCodeGeneratorFromConfig produces a new code generator with the given configuration all available stages +// configuration is used to configure the pipeline. +// idFactory is used to create identifiers for types. +// log is captured by stages to log messages. func NewCodeGeneratorFromConfig( configuration *config.Configuration, idFactory astmodel.IdentifierFactory, + log logr.Logger, ) (*CodeGenerator, error) { result := &CodeGenerator{ configuration: configuration, - pipeline: createAllPipelineStages(idFactory, configuration), + pipeline: createAllPipelineStages(idFactory, configuration, log), } return result, nil } -func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration *config.Configuration) []*pipeline.Stage { +// createAllPipelineStages creates all the stages of the pipeline +// idFactory is used to create identifiers for types. +// configuration is used to configure the pipeline. +// log is captured by stages to log messages. +func createAllPipelineStages( + idFactory astmodel.IdentifierFactory, + configuration *config.Configuration, + log logr.Logger, +) []*pipeline.Stage { return []*pipeline.Stage{ // Import Swagger data: pipeline.LoadTypes(idFactory, configuration), @@ -140,7 +161,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration // This is currently also run as part of RemoveEmbeddedResources and so is technically not needed here, // but we include it to hedge against future changes - pipeline.RemoveEmptyObjects(), + pipeline.RemoveEmptyObjects(log), pipeline.VerifyNoErroredTypes(), @@ -238,8 +259,15 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration } // Generate produces the Go code corresponding to the configured JSON schema in the given output folder -func (generator *CodeGenerator) Generate(ctx context.Context) error { - klog.V(1).Infof("Generator version: %s", version.BuildVersion) +// ctx is used to cancel the generation process. +// log is used to log progress. +func (generator *CodeGenerator) Generate( + ctx context.Context, + log logr.Logger, +) error { + log.V(1).Info( + "ASO Code Generator", + "version", version.BuildVersion) if generator.debugReporter != nil { // Generate a diagram containing our stages @@ -253,12 +281,8 @@ func (generator *CodeGenerator) Generate(ctx context.Context) error { state := pipeline.NewState() for i, stage := range generator.pipeline { - klog.V(0).Infof( - "%d/%d: %s", - i+1, // Computers count from 0, people from 1 - len(generator.pipeline), - stage.Description()) - + stageDescription := fmt.Sprintf("%d/%d: %s", i+1, len(generator.pipeline), stage.Description()) + log.Info(stageDescription) start := time.Now() newState, err := stage.Run(ctx, state) @@ -276,7 +300,7 @@ func (generator *CodeGenerator) Generate(ctx context.Context) error { return errors.Errorf("all type definitions removed by stage %s", stage.Id()) } - generator.logStateChange(state, newState) + generator.logStateChange(state, newState, log) if generator.debugReporter != nil { err := generator.debugReporter.ReportStage(i, stage.Description(), newState) @@ -286,36 +310,41 @@ func (generator *CodeGenerator) Generate(ctx context.Context) error { } duration := time.Since(start).Round(time.Millisecond) - klog.V(0).Infof( - "%d/%d: %s, completed in %s", - i+1, // Computers count from 0, people from 1 - len(generator.pipeline), - stage.Description(), - duration) + log.Info( + stageDescription, + "elapsed", duration) state = newState } if err := state.CheckFinalState(); err != nil { - klog.Info("Failed") + log.Error(err, "Final state check failed") return err } - klog.Info("Finished") + log.Info("Finished") return nil } -func (generator *CodeGenerator) logStateChange(former *pipeline.State, later *pipeline.State) { +func (generator *CodeGenerator) logStateChange( + former *pipeline.State, + later *pipeline.State, + log logr.Logger, +) { defsAdded := later.Definitions().Except(former.Definitions()) defsRemoved := former.Definitions().Except(later.Definitions()) - if len(defsAdded) > 0 && len(defsRemoved) > 0 { - klog.V(1).Infof("Added %d, removed %d type definitions", len(defsAdded), len(defsRemoved)) - } else if len(defsAdded) > 0 { - klog.V(1).Infof("Added %d type definitions", len(defsAdded)) - } else if len(defsRemoved) > 0 { - klog.V(1).Infof("Removed %d type definitions", len(defsRemoved)) + if len(defsAdded) > 0 { + log.V(1).Info( + "Stage added type definitions", + "count", len(defsAdded)) + } + + if len(defsRemoved) > 0 { + log.V(1).Info( + "Stage removed type definitions", + "count", len(defsRemoved)) } } From 5326e7b2c1105da7fc5c6f55b937383551258885 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 20 Apr 2023 15:07:49 +1200 Subject: [PATCH 03/21] Update pipeline stages --- .../internal/astmodel/property_set.go | 14 ++++ .../internal/codegen/code_generator.go | 65 ++++++++-------- .../embeddedresources/remove_empty_objects.go | 39 +++++++--- .../codegen/embeddedresources/remover.go | 7 +- .../internal/codegen/pipeline/add_secrets.go | 3 - ...y_cross_resource_references_from_config.go | 30 ++++++-- .../codegen/pipeline/apply_export_filters.go | 14 ++-- .../apply_kubernetes_resource_interface.go | 12 ++- .../pipeline/apply_property_rewrites.go | 9 ++- .../convert_allof_and_oneof_to_objects.go | 17 +++-- .../codegen/pipeline/create_arm_types.go | 29 ++++--- .../codegen/pipeline/create_arm_types_test.go | 3 +- .../codegen/pipeline/export_generated_code.go | 54 +++++++++---- .../internal/codegen/pipeline/load_types.go | 19 ++++- .../pipeline/remove_embedded_resources.go | 12 ++- .../remove_embedded_resources_test.go | 9 ++- .../codegen/pipeline/remove_empty_objects.go | 6 +- .../internal/config/type_transformer.go | 20 +++++ .../kubernetes_resource_interface.go | 18 +++-- .../jsonast/schema_abstraction_openapi.go | 21 ++++-- .../schema_abstraction_openapi_test.go | 46 ++++++++++-- .../jsonast/swagger_type_extractor.go | 75 +++++++++++++------ 22 files changed, 369 insertions(+), 153 deletions(-) diff --git a/v2/tools/generator/internal/astmodel/property_set.go b/v2/tools/generator/internal/astmodel/property_set.go index 2e68dade474..12d193a12c0 100644 --- a/v2/tools/generator/internal/astmodel/property_set.go +++ b/v2/tools/generator/internal/astmodel/property_set.go @@ -9,6 +9,7 @@ import ( "sort" "golang.org/x/exp/maps" + kerrors "k8s.io/apimachinery/pkg/util/errors" ) // PropertySet wraps a set of property definitions, indexed by name, along with some convenience methods @@ -24,6 +25,7 @@ type ReadOnlyPropertySet interface { Equals(other ReadOnlyPropertySet, overrides EqualityOverrides) bool First() *PropertyDefinition ForEach(func(def *PropertyDefinition)) + ForEachError(func(def *PropertyDefinition) error) error IsEmpty() bool Len() int } @@ -104,6 +106,18 @@ func (p PropertySet) ForEach(f func(*PropertyDefinition)) { } } +func (p PropertySet) ForEachError(f func(*PropertyDefinition) error) error { + var errs []error + for _, v := range p { + err := f(v) + if err != nil { + errs = append(errs, err) + } + } + + return kerrors.NewAggregate(errs) +} + func (p PropertySet) Len() int { return len(p) } diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index 8ec863a4688..3da22c09f3d 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -104,7 +104,7 @@ func createAllPipelineStages( ) []*pipeline.Stage { return []*pipeline.Stage{ // Import Swagger data: - pipeline.LoadTypes(idFactory, configuration), + pipeline.LoadTypes(idFactory, configuration, log), // Assemble actual one-of types from roots and leaves pipeline.AssembleOneOfTypes(idFactory), @@ -129,7 +129,7 @@ func createAllPipelineStages( // Apply property type rewrites from the config file // Must come after NameTypesForCRD ('nameTypes') and ConvertAllOfAndOneOfToObjects ('allof-anyof-objects') so // that objects are all expanded - pipeline.ApplyPropertyRewrites(configuration), + pipeline.ApplyPropertyRewrites(configuration, log), pipeline.ApplyIsResourceOverrides(configuration), pipeline.FixIDFields(), @@ -146,11 +146,11 @@ func createAllPipelineStages( pipeline.StripUnreferencedTypeDefinitions(), pipeline.AssertTypesCollectionValid(), - pipeline.RemoveEmbeddedResources(configuration).UsedFor(pipeline.ARMTarget), + pipeline.RemoveEmbeddedResources(configuration, log).UsedFor(pipeline.ARMTarget), // Apply export filters before generating // ARM types for resources etc: - pipeline.ApplyExportFilters(configuration), + pipeline.ApplyExportFilters(configuration, log), pipeline.AddAPIVersionEnums(), pipeline.RemoveTypeAliases(), @@ -172,7 +172,7 @@ func createAllPipelineStages( pipeline.FixOptionalCollectionAliases(), - pipeline.ApplyCrossResourceReferencesFromConfig(configuration).UsedFor(pipeline.ARMTarget), + pipeline.ApplyCrossResourceReferencesFromConfig(configuration, log).UsedFor(pipeline.ARMTarget), pipeline.TransformCrossResourceReferences(configuration, idFactory).UsedFor(pipeline.ARMTarget), pipeline.TransformCrossResourceReferencesToString().UsedFor(pipeline.CrossplaneTarget), pipeline.AddSecrets(configuration).UsedFor(pipeline.ARMTarget), @@ -187,11 +187,11 @@ func createAllPipelineStages( pipeline.ReportOnTypesAndVersions(configuration).UsedFor(pipeline.ARMTarget), // TODO: For now only used for ARM - pipeline.CreateARMTypes(idFactory).UsedFor(pipeline.ARMTarget), + pipeline.CreateARMTypes(idFactory, log).UsedFor(pipeline.ARMTarget), pipeline.PruneResourcesWithLifecycleOwnedByParent(configuration).UsedFor(pipeline.ARMTarget), pipeline.MakeOneOfDiscriminantRequired().UsedFor(pipeline.ARMTarget), pipeline.ApplyARMConversionInterface(idFactory).UsedFor(pipeline.ARMTarget), - pipeline.ApplyKubernetesResourceInterface(idFactory).UsedFor(pipeline.ARMTarget), + pipeline.ApplyKubernetesResourceInterface(idFactory, log).UsedFor(pipeline.ARMTarget), // Effects the "flatten" property of Properties: pipeline.FlattenProperties(), @@ -249,7 +249,7 @@ func createAllPipelineStages( pipeline.DetectSkippingProperties().UsedFor(pipeline.ARMTarget), pipeline.DeleteGeneratedCode(configuration.FullTypesOutputPath()), - pipeline.ExportPackages(configuration.FullTypesOutputPath(), configuration.EmitDocFiles), + pipeline.ExportPackages(configuration.FullTypesOutputPath(), configuration.EmitDocFiles, log), pipeline.ExportControllerResourceRegistrations(idFactory, configuration.FullTypesRegistrationOutputFilePath()).UsedFor(pipeline.ARMTarget), @@ -300,7 +300,28 @@ func (generator *CodeGenerator) Generate( return errors.Errorf("all type definitions removed by stage %s", stage.Id()) } - generator.logStateChange(state, newState, log) + duration := time.Since(start).Round(time.Millisecond) + + defsAdded := newState.Definitions().Except(state.Definitions()) + defsRemoved := state.Definitions().Except(newState.Definitions()) + + if len(defsAdded) > 0 && len(defsRemoved) > 0 { + log.Info( + stageDescription, + "elapsed", duration, + "added", len(defsAdded), + "removed", len(defsRemoved)) + } else if len(defsAdded) > 0 { + log.Info( + stageDescription, + "elapsed", duration, + "added", len(defsAdded)) + } else if len(defsRemoved) > 0 { + log.Info( + stageDescription, + "elapsed", duration, + "removed", len(defsRemoved)) + } if generator.debugReporter != nil { err := generator.debugReporter.ReportStage(i, stage.Description(), newState) @@ -309,11 +330,6 @@ func (generator *CodeGenerator) Generate( } } - duration := time.Since(start).Round(time.Millisecond) - log.Info( - stageDescription, - "elapsed", duration) - state = newState } @@ -327,27 +343,6 @@ func (generator *CodeGenerator) Generate( return nil } -func (generator *CodeGenerator) logStateChange( - former *pipeline.State, - later *pipeline.State, - log logr.Logger, -) { - defsAdded := later.Definitions().Except(former.Definitions()) - defsRemoved := former.Definitions().Except(later.Definitions()) - - if len(defsAdded) > 0 { - log.V(1).Info( - "Stage added type definitions", - "count", len(defsAdded)) - } - - if len(defsRemoved) > 0 { - log.V(1).Info( - "Stage removed type definitions", - "count", len(defsRemoved)) - } -} - // RemoveStages will remove all stages from the pipeline with the given ids. // Only available for test builds. // Will panic if you specify an unknown id. diff --git a/v2/tools/generator/internal/codegen/embeddedresources/remove_empty_objects.go b/v2/tools/generator/internal/codegen/embeddedresources/remove_empty_objects.go index fbed16d58a2..88ef0da1726 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/remove_empty_objects.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/remove_empty_objects.go @@ -6,23 +6,26 @@ package embeddedresources import ( + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) -func RemoveEmptyObjects(definitions astmodel.TypeDefinitionSet) (astmodel.TypeDefinitionSet, error) { +func RemoveEmptyObjects( + definitions astmodel.TypeDefinitionSet, + log logr.Logger, +) (astmodel.TypeDefinitionSet, error) { result := definitions for { - toRemove := findEmptyObjectTypes(result) + toRemove := findEmptyObjectTypes(result, log) if len(toRemove) == 0 { break } var err error - result, err = removeReferencesToTypes(result, toRemove) + result, err = removeReferencesToTypes(result, toRemove, log) if err != nil { return nil, err } @@ -31,7 +34,10 @@ func RemoveEmptyObjects(definitions astmodel.TypeDefinitionSet) (astmodel.TypeDe return result, nil } -func findEmptyObjectTypes(definitions astmodel.TypeDefinitionSet) astmodel.TypeNameSet { +func findEmptyObjectTypes( + definitions astmodel.TypeDefinitionSet, + log logr.Logger, +) astmodel.TypeNameSet { result := astmodel.NewTypeNameSet() for _, def := range definitions { @@ -49,16 +55,23 @@ func findEmptyObjectTypes(definitions astmodel.TypeDefinitionSet) astmodel.TypeN continue } - klog.V(4).Infof("Removing %q as it has no properties", def.Name()) + log.V(1).Info( + "Removing empty type", + "name", def.Name()) + result.Add(def.Name()) } return result } -func removeReferencesToTypes(definitions astmodel.TypeDefinitionSet, toRemove astmodel.TypeNameSet) (astmodel.TypeDefinitionSet, error) { +func removeReferencesToTypes( + definitions astmodel.TypeDefinitionSet, + toRemove astmodel.TypeNameSet, + log logr.Logger, +) (astmodel.TypeDefinitionSet, error) { result := make(astmodel.TypeDefinitionSet) - visitor := makeRemovedTypeVisitor(toRemove) + visitor := makeRemovedTypeVisitor(toRemove, log) for _, def := range definitions { if toRemove.Contains(def.Name()) { @@ -79,7 +92,10 @@ type visitorCtx struct { typeName *astmodel.TypeName } -func makeRemovedTypeVisitor(toRemove astmodel.TypeNameSet) astmodel.TypeVisitor { +func makeRemovedTypeVisitor( + toRemove astmodel.TypeNameSet, + log logr.Logger, +) astmodel.TypeVisitor { // This is basically copied from IdentityVisitOfObjectType, but since it has/needs a per-property context we can't use that removeReferencesToEmptyTypes := func(this *astmodel.TypeVisitor, it *astmodel.ObjectType, ctx interface{}) (astmodel.Type, error) { // just map the property types @@ -93,7 +109,10 @@ func makeRemovedTypeVisitor(toRemove astmodel.TypeNameSet) astmodel.TypeVisitor } else if ctx.typeName == nil || !toRemove.Contains(*ctx.typeName) { newProps = append(newProps, prop.WithType(p)) } else if toRemove.Contains(*ctx.typeName) { - klog.V(4).Infof("Removing property %q (referencing %q) as the type has no properties", prop.PropertyName(), *ctx.typeName) + log.V(1).Info( + "Removing reference to empty type", + "property", prop.PropertyName(), + "referencing", *ctx.typeName) } }) diff --git a/v2/tools/generator/internal/codegen/embeddedresources/remover.go b/v2/tools/generator/internal/codegen/embeddedresources/remover.go index 21688de717a..b700e92c244 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/remover.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/remover.go @@ -8,6 +8,7 @@ package embeddedresources import ( "fmt" + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" @@ -103,7 +104,9 @@ func MakeEmbeddedResourceRemover(configuration *config.Configuration, definition } // RemoveEmbeddedResources removes any embedded resources according to the -func (e EmbeddedResourceRemover) RemoveEmbeddedResources() (astmodel.TypeDefinitionSet, error) { +func (e EmbeddedResourceRemover) RemoveEmbeddedResources( + log logr.Logger, +) (astmodel.TypeDefinitionSet, error) { result := make(astmodel.TypeDefinitionSet) originalNames := make(map[astmodel.TypeName]embeddedResourceTypeName) @@ -135,7 +138,7 @@ func (e EmbeddedResourceRemover) RemoveEmbeddedResources() (astmodel.TypeDefinit return nil, err } - return RemoveEmptyObjects(result) + return RemoveEmptyObjects(result, log) } func (e EmbeddedResourceRemover) makeEmbeddedResourceRemovalTypeVisitor() astmodel.TypeVisitor { diff --git a/v2/tools/generator/internal/codegen/pipeline/add_secrets.go b/v2/tools/generator/internal/codegen/pipeline/add_secrets.go index 5c167f162bc..838e4f798ba 100644 --- a/v2/tools/generator/internal/codegen/pipeline/add_secrets.go +++ b/v2/tools/generator/internal/codegen/pipeline/add_secrets.go @@ -9,7 +9,6 @@ import ( "context" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -79,8 +78,6 @@ func applyConfigSecretOverrides(config *config.Configuration, definitions astmod // Verify that all 'isSecret' modifiers are consumed before returning the result err := config.VerifyIsSecretConsumed() if err != nil { - klog.Error(err) - return nil, errors.Wrap( err, "Found unused $isSecret configurations; these need to be fixed or removed.") diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go index 3f4665ec6c8..46010b9692c 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_cross_resource_references_from_config.go @@ -8,9 +8,9 @@ package pipeline import ( "context" + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -28,7 +28,10 @@ const ( const ApplyCrossResourceReferencesFromConfigStageID = "applyCrossResourceReferencesFromConfig" // ApplyCrossResourceReferencesFromConfig replaces cross resource references from the configuration with astmodel.ARMID. -func ApplyCrossResourceReferencesFromConfig(configuration *config.Configuration) *Stage { +func ApplyCrossResourceReferencesFromConfig( + configuration *config.Configuration, + log logr.Logger, +) *Stage { return NewStage( ApplyCrossResourceReferencesFromConfigStageID, "Replace cross-resource references in the config with astmodel.ARMID", @@ -88,7 +91,7 @@ func ApplyCrossResourceReferencesFromConfig(configuration *config.Configuration) return ARMIDPropertyClassificationUnspecified } - visitor := MakeARMIDPropertyTypeVisitor(isCrossResourceReference) + visitor := MakeARMIDPropertyTypeVisitor(isCrossResourceReference, log) for _, def := range state.Definitions() { // Skip Status types // TODO: we need flags @@ -115,8 +118,6 @@ func ApplyCrossResourceReferencesFromConfig(configuration *config.Configuration) err = configuration.VerifyARMReferencesConsumed() if err != nil { - klog.Error(err) - return nil, errors.Wrap( err, "Found unused $armReference configurations; these need to be fixed or removed.") @@ -135,7 +136,10 @@ type ARMIDPropertyTypeVisitor struct { isPropertyAnARMReference crossResourceReferenceChecker } -func MakeARMIDPropertyTypeVisitor(referenceChecker crossResourceReferenceChecker) ARMIDPropertyTypeVisitor { +func MakeARMIDPropertyTypeVisitor( + referenceChecker crossResourceReferenceChecker, + log logr.Logger, +) ARMIDPropertyTypeVisitor { visitor := ARMIDPropertyTypeVisitor{ isPropertyAnARMReference: referenceChecker, } @@ -147,10 +151,20 @@ func MakeARMIDPropertyTypeVisitor(referenceChecker crossResourceReferenceChecker it.Properties().ForEach(func(prop *astmodel.PropertyDefinition) { classification := visitor.isPropertyAnARMReference(typeName, prop) if classification == ARMIDPropertyClassificationSet { - klog.V(4).Infof("Transforming \"%s.%s\" field into astmodel.ARMID", typeName, prop.PropertyName()) + log.V(1).Info( + "Transforming property", + "definition", typeName, + "property", prop.PropertyName(), + "was", prop.PropertyType(), + "now", "astmodel.ARMID") prop = makeARMIDProperty(prop) } else if classification == ARMIDPropertyClassificationUnset { - klog.V(4).Infof("Transforming \"%s.%s\" field into string from astmodel.ARMID", typeName, prop.PropertyName()) + log.V(1).Info( + "Transforming property", + "definition", typeName, + "property", prop.PropertyName(), + "was", prop.PropertyType(), + "now", "string") prop = unsetARMIDProperty(prop) } diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_export_filters.go b/v2/tools/generator/internal/codegen/pipeline/apply_export_filters.go index 39c96765105..69c67d88709 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_export_filters.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_export_filters.go @@ -8,9 +8,9 @@ package pipeline import ( "context" + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -19,12 +19,15 @@ import ( const ApplyExportFiltersStageID = "filterTypes" // ApplyExportFilters creates a Stage to reduce our set of types for export -func ApplyExportFilters(configuration *config.Configuration) *Stage { +func ApplyExportFilters( + configuration *config.Configuration, + log logr.Logger, +) *Stage { stage := NewStage( ApplyExportFiltersStageID, "Apply export filters to reduce the number of generated types", func(ctx context.Context, state *State) (*State, error) { - return filterTypes(configuration, state) + return filterTypes(configuration, state, log) }) stage.RequiresPostrequisiteStages(VerifyNoErroredTypesStageID) @@ -35,6 +38,7 @@ func ApplyExportFilters(configuration *config.Configuration) *Stage { func filterTypes( configuration *config.Configuration, state *State, + log logr.Logger, ) (*State, error) { resourcesToExport := make(astmodel.TypeDefinitionSet) var errs []error @@ -48,11 +52,11 @@ func filterTypes( } if !export { - klog.V(3).Infof("Skipping resource %s", defName) + log.V(1).Info("Skipping resource", "resource", defName) continue } - klog.V(3).Infof("Exporting resource %s and related types", defName) + log.V(1).Info("Exporting resource", "resource", defName) resourcesToExport.Add(def) } diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_kubernetes_resource_interface.go b/v2/tools/generator/internal/codegen/pipeline/apply_kubernetes_resource_interface.go index 2b9ab371091..6214fe0404a 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_kubernetes_resource_interface.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_kubernetes_resource_interface.go @@ -8,6 +8,7 @@ package pipeline import ( "context" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -17,7 +18,10 @@ import ( const ApplyKubernetesResourceInterfaceStageID = "applyKubernetesResourceInterface" // ApplyKubernetesResourceInterface ensures that every Resource implements the KubernetesResource interface -func ApplyKubernetesResourceInterface(idFactory astmodel.IdentifierFactory) *Stage { +func ApplyKubernetesResourceInterface( + idFactory astmodel.IdentifierFactory, + log logr.Logger, +) *Stage { return NewStage( ApplyKubernetesResourceInterfaceStageID, "Add the KubernetesResource interface to every resource", @@ -26,7 +30,11 @@ func ApplyKubernetesResourceInterface(idFactory astmodel.IdentifierFactory) *Sta for typeName, typeDef := range astmodel.FindResourceDefinitions(state.Definitions()) { resource := typeDef.Type().(*astmodel.ResourceType) - newResource, err := interfaces.AddKubernetesResourceInterfaceImpls(typeDef, idFactory, state.Definitions()) + newResource, err := interfaces.AddKubernetesResourceInterfaceImpls( + typeDef, + idFactory, + state.Definitions(), + log) if err != nil { return nil, errors.Wrapf(err, "couldn't implement Kubernetes resource interface for %q", typeName) } diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go b/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go index 602df03f16d..71223702e4b 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go @@ -8,7 +8,7 @@ package pipeline import ( "context" - "k8s.io/klog/v2" + "github.com/go-logr/logr" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -17,7 +17,10 @@ import ( // ApplyPropertyRewrites applies any typeTransformers for properties. // It is its own pipeline stage so that we can apply it after the allOf/oneOf types have // been "lowered" to objects. -func ApplyPropertyRewrites(config *config.Configuration) *Stage { +func ApplyPropertyRewrites( + config *config.Configuration, + log logr.Logger, +) *Stage { stage := NewLegacyStage( "propertyRewrites", "Modify property types using configured transforms", @@ -32,7 +35,7 @@ func ApplyPropertyRewrites(config *config.Configuration) *Stage { transformations := config.TransformTypeProperties(name, objectType) for _, transformation := range transformations { - klog.V(3).Infof("Transforming %s", transformation) + transformation.Log(log) objectType = transformation.NewType } diff --git a/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go b/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go index 48430ebddb1..1e90f7aaa75 100644 --- a/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go +++ b/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go @@ -12,7 +12,6 @@ import ( "github.com/pkg/errors" "golang.org/x/exp/maps" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -585,7 +584,10 @@ func max(left, right int) int { return right } -func (s synthesizer) handleObjectObject(leftObj *astmodel.ObjectType, rightObj *astmodel.ObjectType) (astmodel.Type, error) { +func (s synthesizer) handleObjectObject( + leftObj *astmodel.ObjectType, + rightObj *astmodel.ObjectType, +) (astmodel.Type, error) { leftProperties := leftObj.Properties() rightProperties := rightObj.Properties() mergedProps := make(map[astmodel.PropertyName]*astmodel.PropertyDefinition, max(leftProperties.Len(), rightProperties.Len())) @@ -594,18 +596,17 @@ func (s synthesizer) handleObjectObject(leftObj *astmodel.ObjectType, rightObj * mergedProps[p.PropertyName()] = p }) - rightProperties.ForEach(func(p *astmodel.PropertyDefinition) { + err := rightProperties.ForEachError(func(p *astmodel.PropertyDefinition) error { existingProp, ok := mergedProps[p.PropertyName()] if !ok { // Property doesn't already exist, so just add it mergedProps[p.PropertyName()] = p - return // continue + return nil } newType, err := s.intersectTypes(existingProp.PropertyType(), p.PropertyType()) if err != nil { - klog.Errorf("unable to combine properties: %s (%s)", p.PropertyName(), err) - return // continue + return errors.Wrapf(err, "unable to combine properties: %s", p.PropertyName()) } // TODO: need to handle merging requiredness and tags and... @@ -619,7 +620,11 @@ func (s synthesizer) handleObjectObject(leftObj *astmodel.ObjectType, rightObj * } mergedProps[p.PropertyName()] = newProp + return nil }) + if err != nil { + return nil, errors.Wrapf(err, "unable to combine properties in handle object-object") + } // flatten properties := maps.Values(mergedProps) diff --git a/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go b/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go index 3568be2decc..7c8530125b0 100644 --- a/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go @@ -8,9 +8,9 @@ package pipeline import ( "context" + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/armconversion" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -22,20 +22,20 @@ const CreateARMTypesStageID = "createArmTypes" // CreateARMTypes walks the type graph and builds new types for communicating // with ARM -func CreateARMTypes(idFactory astmodel.IdentifierFactory) *Stage { - return NewLegacyStage( +func CreateARMTypes(idFactory astmodel.IdentifierFactory, log logr.Logger) *Stage { + return NewStage( CreateARMTypesStageID, "Create types for interaction with ARM", - func(ctx context.Context, definitions astmodel.TypeDefinitionSet) (astmodel.TypeDefinitionSet, error) { - typeCreator := newARMTypeCreator(definitions, idFactory) + func(ctx context.Context, state *State) (*State, error) { + typeCreator := newARMTypeCreator(state.Definitions(), idFactory, log) armTypes, err := typeCreator.createARMTypes() if err != nil { return nil, err } - result := astmodel.TypesDisjointUnion(armTypes, definitions) + newDefs := astmodel.TypesDisjointUnion(armTypes, state.Definitions()) - return result, nil + return state.WithDefinitions(newDefs), nil }) } @@ -54,9 +54,13 @@ type armTypeCreator struct { idFactory astmodel.IdentifierFactory newDefs astmodel.TypeDefinitionSet skipTypes []func(it astmodel.TypeDefinition) bool + log logr.Logger } -func newARMTypeCreator(definitions astmodel.TypeDefinitionSet, idFactory astmodel.IdentifierFactory) *armTypeCreator { +func newARMTypeCreator( + definitions astmodel.TypeDefinitionSet, + idFactory astmodel.IdentifierFactory, + log logr.Logger) *armTypeCreator { return &armTypeCreator{ definitions: definitions, idFactory: idFactory, @@ -64,6 +68,7 @@ func newARMTypeCreator(definitions astmodel.TypeDefinitionSet, idFactory astmode skipTypes: []func(it astmodel.TypeDefinition) bool{ skipUserAssignedIdentity, }, + log: log, } } @@ -179,7 +184,9 @@ func (c *armTypeCreator) createARMTypeDefinition(isSpecType bool, def astmodel.T addOneOfConversionFunctionIfNeeded := func(t *astmodel.ObjectType) (*astmodel.ObjectType, error) { if isOneOf { - klog.V(4).Infof("Type %s is a OneOf type, adding MarshalJSON and UnmarshalJSON", def.Name()) + c.log.Info( + "Adding MarshalJSON and UnmarshalJSON to OneOf", + "type", def.Name()) marshal := functions.NewOneOfJSONMarshalFunction(t, c.idFactory) unmarshal := functions.NewOneOfJSONUnmarshalFunction(t, c.idFactory) return t.WithFunction(marshal).WithFunction(unmarshal), nil @@ -257,7 +264,7 @@ func (c *armTypeCreator) createARMTypeIfNeeded(t astmodel.Type) (astmodel.Type, func (c *armTypeCreator) createARMNameProperty(prop *astmodel.PropertyDefinition, isSpec bool) (*astmodel.PropertyDefinition, error) { if isSpec && prop.HasName("Name") { // all resource Spec Name properties must be strings on their way to ARM - // as nested resources will have the owner etc added to the start: + // as nested resources will have the owner etc. added to the start: return prop.WithType(astmodel.StringType), nil } @@ -414,7 +421,7 @@ func (c *armTypeCreator) convertObjectPropertiesForARM(t *astmodel.ObjectType, i } // Also convert embedded properties if there are any - result = result.WithoutEmbeddedProperties() // Clear them out first so we're starting with a clean slate + result = result.WithoutEmbeddedProperties() // Clear them out first, so we're starting with a clean slate for _, prop := range t.EmbeddedProperties() { for _, handler := range embeddedPropertyHandlers { newProp, err := handler(prop, isSpecType) diff --git a/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go b/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go index 729c1283ae9..ab8229c3ea4 100644 --- a/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -97,7 +98,7 @@ func TestCreateFlattenedARMTypeWithResourceRef_CreatesExpectedConversions(t *tes configuration := config.NewConfiguration() configuration.ObjectModelConfiguration = omc - configToARMIDs := ApplyCrossResourceReferencesFromConfig(configuration) + configToARMIDs := ApplyCrossResourceReferencesFromConfig(configuration, logr.Discard()) crossResourceRefs := TransformCrossResourceReferences(configuration, idFactory) createARMTypes := CreateARMTypes(idFactory) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) diff --git a/v2/tools/generator/internal/codegen/pipeline/export_generated_code.go b/v2/tools/generator/internal/codegen/pipeline/export_generated_code.go index e9e9b409bad..9fde5462ff9 100644 --- a/v2/tools/generator/internal/codegen/pipeline/export_generated_code.go +++ b/v2/tools/generator/internal/codegen/pipeline/export_generated_code.go @@ -14,9 +14,9 @@ import ( "sync" "time" + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -25,7 +25,11 @@ import ( const ExportPackagesStageID = "exportPackages" // ExportPackages creates a Stage to export our generated code as a set of packages -func ExportPackages(outputPath string, emitDocFiles bool) *Stage { +func ExportPackages( + outputPath string, + emitDocFiles bool, + log logr.Logger, +) *Stage { description := fmt.Sprintf("Export packages to %q", outputPath) stage := NewLegacyStage( ExportPackagesStageID, @@ -36,7 +40,7 @@ func ExportPackages(outputPath string, emitDocFiles bool) *Stage { return nil, errors.Wrapf(err, "failed to assign generated definitions to packages") } - err = writeFiles(ctx, packages, outputPath, emitDocFiles) + err = writeFiles(ctx, packages, outputPath, emitDocFiles, log) if err != nil { return nil, errors.Wrapf(err, "unable to write files into %q", outputPath) } @@ -68,7 +72,13 @@ func CreatePackagesForDefinitions(definitions astmodel.TypeDefinitionSet) (map[a return packages, nil } -func writeFiles(ctx context.Context, packages map[astmodel.PackageReference]*astmodel.PackageDefinition, outputPath string, emitDocFiles bool) error { +func writeFiles( + ctx context.Context, + packages map[astmodel.PackageReference]*astmodel.PackageDefinition, + outputPath string, + emitDocFiles bool, + log logr.Logger, +) error { pkgs := make([]*astmodel.PackageDefinition, 0, len(packages)) for _, pkg := range packages { pkgs = append(pkgs, pkg) @@ -83,7 +93,10 @@ func writeFiles(ctx context.Context, packages map[astmodel.PackageReference]*ast }) // emit each package - klog.V(0).Infof("Writing %d packages into %q", len(pkgs), outputPath) + log.Info( + "Writing packages", + "count", len(pkgs), + "outputPath", outputPath) globalProgress := newProgressMeter() groupProgress := newProgressMeter() @@ -108,7 +121,6 @@ func writeFiles(ctx context.Context, packages map[astmodel.PackageReference]*ast // create directory if not already there outputDir := filepath.Join(outputPath, pkg.GroupName, pkg.PackageName) if _, err := os.Stat(outputDir); os.IsNotExist(err) { - klog.V(5).Infof("Creating directory %q\n", outputDir) err = os.MkdirAll(outputDir, 0o700) if err != nil { select { // try to write to errs, ignore if buffer full @@ -127,8 +139,8 @@ func writeFiles(ctx context.Context, packages map[astmodel.PackageReference]*ast } return } else { - globalProgress.LogProgress("", pkg.DefinitionCount(), count) - groupProgress.LogProgress(pkg.GroupName, pkg.DefinitionCount(), count) + globalProgress.LogProgress("", pkg.DefinitionCount(), count, log) + groupProgress.LogProgress(pkg.GroupName, pkg.DefinitionCount(), count, log) } } }() @@ -157,7 +169,7 @@ func writeFiles(ctx context.Context, packages map[astmodel.PackageReference]*ast // log anything leftover globalProgress.mutex.Lock() defer globalProgress.mutex.Unlock() - globalProgress.Log() + globalProgress.Log(log) return nil } @@ -178,7 +190,7 @@ type progressMeter struct { } // Log writes a log message for our progress to this point -func (export *progressMeter) Log() { +func (export *progressMeter) Log(log logr.Logger) { started := export.resetAt export.resetAt = time.Now() @@ -188,22 +200,36 @@ func (export *progressMeter) Log() { elapsed := time.Since(started).Round(time.Millisecond) if export.label != "" { - klog.V(2).Infof("Wrote %d files containing %d definitions for %s in %s", export.files, export.definitions, export.label, elapsed) + log.V(1).Info( + "Wrote files", + "label", export.label, + "files", export.files, + "types", export.definitions, + "elapsed", elapsed) } else { - klog.V(2).Infof("Wrote %d files containing %d definitions in %s", export.files, export.definitions, time.Since(started)) + log.V(1).Info( + "Wrote files", + "files", export.files, + "types", export.definitions, + "elapsed", elapsed) } export.resetAt = time.Now() } // LogProgress accumulates totals until a new label is supplied, when it will write a log message -func (export *progressMeter) LogProgress(label string, definitions int, files int) { +func (export *progressMeter) LogProgress( + label string, + definitions int, + files int, + log logr.Logger, +) { export.mutex.Lock() defer export.mutex.Unlock() if export.label != label { // New group, output our current totals and reset - export.Log() + export.Log(log) export.definitions = 0 export.files = 0 export.resetAt = time.Now() diff --git a/v2/tools/generator/internal/codegen/pipeline/load_types.go b/v2/tools/generator/internal/codegen/pipeline/load_types.go index 22a09fc67c8..a7b196cb37c 100644 --- a/v2/tools/generator/internal/codegen/pipeline/load_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/load_types.go @@ -16,6 +16,7 @@ import ( "strings" "sync" + "github.com/go-logr/logr" "github.com/go-openapi/spec" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -37,14 +38,18 @@ any actions that appear to be ARM resources (have PUT methods with types we can action path). Then for each resource, we use the existing JSON AST parser to extract the status type (the type-definition part of swagger is the same as JSON Schema). */ -func LoadTypes(idFactory astmodel.IdentifierFactory, config *config.Configuration) *Stage { +func LoadTypes( + idFactory astmodel.IdentifierFactory, + config *config.Configuration, + log logr.Logger, +) *Stage { return NewLegacyStage( LoadTypesStageID, "Load all types from Swagger files", func(ctx context.Context, definitions astmodel.TypeDefinitionSet) (astmodel.TypeDefinitionSet, error) { klog.V(1).Infof("Loading Swagger data from %q", config.SchemaRoot) - swaggerTypes, err := loadSwaggerData(ctx, idFactory, config) + swaggerTypes, err := loadSwaggerData(ctx, idFactory, config, log) if err != nil { return nil, errors.Wrapf(err, "unable to load Swagger data") } @@ -292,7 +297,12 @@ func generateSpecTypes(swaggerTypes jsonast.SwaggerTypes) (astmodel.TypeDefiniti return resources, otherTypes, nil } -func loadSwaggerData(ctx context.Context, idFactory astmodel.IdentifierFactory, config *config.Configuration) (jsonast.SwaggerTypes, error) { +func loadSwaggerData( + ctx context.Context, + idFactory astmodel.IdentifierFactory, + config *config.Configuration, + log logr.Logger, +) (jsonast.SwaggerTypes, error) { schemas, err := loadAllSchemas(ctx, config.SchemaRoot, config.LocalPathPrefix(), idFactory, config.Status.Overrides) if err != nil { return jsonast.SwaggerTypes{}, err @@ -309,7 +319,8 @@ func loadSwaggerData(ctx context.Context, idFactory astmodel.IdentifierFactory, schema.Swagger, schemaPath, *schema.Package, // always set during generation - loader) + loader, + log) types, err := extractor.ExtractTypes(ctx) if err != nil { diff --git a/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources.go b/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources.go index 1b1c9eceab5..a192a90e204 100644 --- a/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources.go +++ b/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources.go @@ -8,6 +8,8 @@ package pipeline import ( "context" + "github.com/go-logr/logr" + "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/codegen/embeddedresources" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -16,16 +18,20 @@ import ( // RemoveEmbeddedResourcesStageID is the unique identifier for this pipeline stage const RemoveEmbeddedResourcesStageID = "removeEmbeddedResources" -func RemoveEmbeddedResources(configuration *config.Configuration) *Stage { +func RemoveEmbeddedResources( + configuration *config.Configuration, + log logr.Logger, +) *Stage { return NewLegacyStage( RemoveEmbeddedResourcesStageID, - "Remove properties that point to embedded resources. Only removes structural aspects of embedded resources, Id/ARMId references are retained.", + // Only removes structural aspects of embedded resources, Id/ARMId references are retained. + "Remove properties that point to embedded resources.", func(ctx context.Context, definitions astmodel.TypeDefinitionSet) (astmodel.TypeDefinitionSet, error) { remover, err := embeddedresources.MakeEmbeddedResourceRemover(configuration, definitions) if err != nil { return nil, err } - return remover.RemoveEmbeddedResources() + return remover.RemoveEmbeddedResources(log) }) } diff --git a/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources_test.go b/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources_test.go index 509f1a4e3fc..730b2d21943 100644 --- a/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/remove_embedded_resources_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -185,7 +186,7 @@ func TestGolden_EmbeddedSubresource_IsRemoved(t *testing.T) { // TODO: Take this bit and put it into a helper? // Define stages to run configuration := config.NewConfiguration() - removeEmbedded := RemoveEmbeddedResources(configuration) + removeEmbedded := RemoveEmbeddedResources(configuration, logr.Discard()) state, err := RunTestPipeline( NewState().WithDefinitions(defs), @@ -204,7 +205,7 @@ func TestGolden_EmbeddedResource_IsRemovedRetainsId(t *testing.T) { // Define stages to run configuration := config.NewConfiguration() - removeEmbedded := RemoveEmbeddedResources(configuration) + removeEmbedded := RemoveEmbeddedResources(configuration, logr.Discard()) state, err := RunTestPipeline( NewState().WithDefinitions(defs), @@ -228,7 +229,7 @@ func TestGolden_EmbeddedResourcesWithMultipleEmbeddings_AllEmbeddingsAreRemovedA // Define stages to run configuration := config.NewConfiguration() - removeEmbedded := RemoveEmbeddedResources(configuration) + removeEmbedded := RemoveEmbeddedResources(configuration, logr.Discard()) state, err := RunTestPipeline( NewState().WithDefinitions(defs), @@ -284,7 +285,7 @@ func TestGolden_EmbeddedResourceWithCyclesAndResourceLookalikes_RemovesCycles(t // Define stages to run configuration := config.NewConfiguration() - removeEmbedded := RemoveEmbeddedResources(configuration) + removeEmbedded := RemoveEmbeddedResources(configuration, logr.Discard()) state, err := RunTestPipeline( NewState().WithDefinitions(defs), diff --git a/v2/tools/generator/internal/codegen/pipeline/remove_empty_objects.go b/v2/tools/generator/internal/codegen/pipeline/remove_empty_objects.go index a85dc1fdb21..36c000f1492 100644 --- a/v2/tools/generator/internal/codegen/pipeline/remove_empty_objects.go +++ b/v2/tools/generator/internal/codegen/pipeline/remove_empty_objects.go @@ -8,18 +8,20 @@ package pipeline import ( "context" + "github.com/go-logr/logr" + "github.com/Azure/azure-service-operator/v2/tools/generator/internal/codegen/embeddedresources" ) const RemoveEmptyObjectsStageId = "removeEmptyObjects" // RemoveEmptyObjects removes Definitions which are empty (an object type with no properties) -func RemoveEmptyObjects() *Stage { +func RemoveEmptyObjects(log logr.Logger) *Stage { return NewStage( RemoveEmptyObjectsStageId, "Remove empty Objects", func(ctx context.Context, state *State) (*State, error) { - updatedDefs, err := embeddedresources.RemoveEmptyObjects(state.Definitions()) + updatedDefs, err := embeddedresources.RemoveEmptyObjects(state.Definitions(), log) if err != nil { return nil, err } diff --git a/v2/tools/generator/internal/config/type_transformer.go b/v2/tools/generator/internal/config/type_transformer.go index bada80c8fb2..61eb24690de 100644 --- a/v2/tools/generator/internal/config/type_transformer.go +++ b/v2/tools/generator/internal/config/type_transformer.go @@ -9,6 +9,7 @@ import ( "fmt" "strings" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -333,6 +334,25 @@ func (r PropertyTransformResult) String() string { ) } +// Log creates a log message for the transformation +func (r PropertyTransformResult) Log(log logr.Logger) { + if r.Removed { + log.V(2).Info( + "Removing property", + "type", r.TypeName, + "property", r.Property, + "because", r.Because) + return + } + + log.V(1).Info( + "Transforming property", + "type", r.TypeName, + "property", r.Property, + "newType", r.NewPropertyType.String(), + "because", r.Because) +} + func (transformer *TypeTransformer) RequiredPropertiesWereMatched() error { // If this transformer applies to entire types (instead of just properties on types), we just defer to // transformer.MatchedRequiredTypes diff --git a/v2/tools/generator/internal/interfaces/kubernetes_resource_interface.go b/v2/tools/generator/internal/interfaces/kubernetes_resource_interface.go index b566f503f6e..fb7c170e1f1 100644 --- a/v2/tools/generator/internal/interfaces/kubernetes_resource_interface.go +++ b/v2/tools/generator/internal/interfaces/kubernetes_resource_interface.go @@ -10,8 +10,8 @@ import ( "go/token" "github.com/dave/dst" + "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -24,6 +24,7 @@ func AddKubernetesResourceInterfaceImpls( resourceDef astmodel.TypeDefinition, idFactory astmodel.IdentifierFactory, definitions astmodel.TypeDefinitionSet, + log logr.Logger, ) (*astmodel.ResourceType, error) { resolved, err := definitions.ResolveResourceSpecAndStatus(resourceDef) if err != nil { @@ -47,7 +48,12 @@ func AddKubernetesResourceInterfaceImpls( return nil, errors.Errorf("resource spec doesn't have %q property", astmodel.AzureNameProperty) } - getNameFunction, setNameFunction, err := getAzureNameFunctionsForType(&r, spec, azureNameProp.PropertyType(), definitions) + getNameFunction, setNameFunction, err := getAzureNameFunctionsForType( + &r, + spec, + azureNameProp.PropertyType(), + definitions, + log) if err != nil { return nil, err } @@ -119,13 +125,15 @@ func AddKubernetesResourceInterfaceImpls( return r, nil } -// note that this can, as a side-effect, update the resource type +// note that this can, as a side effect, update the resource type // it is a bit ugly! func getAzureNameFunctionsForType( r **astmodel.ResourceType, spec *astmodel.ObjectType, t astmodel.Type, - definitions astmodel.TypeDefinitionSet) (functions.ObjectFunctionHandler, functions.ObjectFunctionHandler, error) { + definitions astmodel.TypeDefinitionSet, + log logr.Logger, +) (functions.ObjectFunctionHandler, functions.ObjectFunctionHandler, error) { if opt, ok := astmodel.AsOptionalType(t); ok { t = opt.BaseType() @@ -146,7 +154,7 @@ func getAzureNameFunctionsForType( return fixedValueGetAzureNameFunction("default"), nil, nil // no SetAzureName for this case } else { // ignoring for now: - klog.Warningf("unable to handle pattern in Name property: %s", validations.Patterns[0].String()) + log.V(1).Info("ignoring pattern validation on Name property", "pattern", validations.Patterns[0].String()) return getStringAzureNameFunction, setStringAzureNameFunction, nil } } else { diff --git a/v2/tools/generator/internal/jsonast/schema_abstraction_openapi.go b/v2/tools/generator/internal/jsonast/schema_abstraction_openapi.go index 0976f33b068..d18f3b5c77f 100644 --- a/v2/tools/generator/internal/jsonast/schema_abstraction_openapi.go +++ b/v2/tools/generator/internal/jsonast/schema_abstraction_openapi.go @@ -7,17 +7,18 @@ package jsonast import ( "fmt" - "github.com/Azure/azure-service-operator/v2/internal/set" "math/big" "net/url" "path/filepath" "regexp" "strings" + "github.com/Azure/azure-service-operator/v2/internal/set" + "github.com/go-logr/logr" + "github.com/go-openapi/jsonpointer" "github.com/go-openapi/spec" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -30,6 +31,7 @@ type OpenAPISchema struct { outputPackage astmodel.LocalPackageReference idFactory astmodel.IdentifierFactory loader OpenAPIFileLoader + log logr.Logger } // MakeOpenAPISchema wraps a spec.Swagger to conform to the Schema abstraction @@ -39,14 +41,18 @@ func MakeOpenAPISchema( fileName string, outputPackage astmodel.LocalPackageReference, idFactory astmodel.IdentifierFactory, - cache OpenAPIFileLoader) Schema { + cache OpenAPIFileLoader, + log logr.Logger, +) Schema { return &OpenAPISchema{ name: name, inner: schema, fileName: fileName, outputPackage: outputPackage, idFactory: idFactory, - loader: cache} + loader: cache, + log: log, + } } func (schema *OpenAPISchema) withNewSchema(newSchema spec.Schema) Schema { @@ -188,7 +194,9 @@ func (schema *OpenAPISchema) pattern() *regexp.Regexp { result, err := regexp.Compile(p) if err != nil { - klog.Warningf("Unable to compile regexp, ignoring: %s", p) // use %s instead of %q or everything gets re-escaped + schema.log.V(1).Info( + "Ignoring regexp we can't compile", + "pattern", p) return nil } @@ -401,7 +409,8 @@ func (schema *OpenAPISchema) refSchema() Schema { fileName, outputPackage, schema.idFactory, - schema.loader) + schema.loader, + schema.log) } // findFileForRef identifies the schema path for a ref, relative to the give schema path diff --git a/v2/tools/generator/internal/jsonast/schema_abstraction_openapi_test.go b/v2/tools/generator/internal/jsonast/schema_abstraction_openapi_test.go index be7a7f5898f..c55df7229f5 100644 --- a/v2/tools/generator/internal/jsonast/schema_abstraction_openapi_test.go +++ b/v2/tools/generator/internal/jsonast/schema_abstraction_openapi_test.go @@ -10,6 +10,7 @@ import ( "runtime" "testing" + "github.com/go-logr/logr" "github.com/go-openapi/spec" . "github.com/onsi/gomega" @@ -48,7 +49,14 @@ func Test_CanExtractTypeNameFromSameFile(t *testing.T) { }, } - wrappedSchema := MakeOpenAPISchema("name", schema, schemaPath, schemaPackage, astmodel.NewIdentifierFactory(), loader) + wrappedSchema := MakeOpenAPISchema( + "name", + schema, + schemaPath, + schemaPackage, + astmodel.NewIdentifierFactory(), + loader, + logr.Discard()) typeName, err := wrappedSchema.refTypeName() g.Expect(err).ToNot(HaveOccurred()) @@ -91,7 +99,14 @@ func Test_CanExtractTypeNameFromDifferentFile_AndInheritPackage(t *testing.T) { }, } - wrappedSchema := MakeOpenAPISchema("name", schema, schemaPath, schemaPackage, astmodel.NewIdentifierFactory(), loader) + wrappedSchema := MakeOpenAPISchema( + "name", + schema, + schemaPath, + schemaPackage, + astmodel.NewIdentifierFactory(), + loader, + logr.Discard()) typeName, err := wrappedSchema.refTypeName() g.Expect(err).ToNot(HaveOccurred()) @@ -136,7 +151,14 @@ func Test_CanExtractTypeNameFromDifferentFile_AndUsePresetPackage(t *testing.T) }, } - wrappedSchema := MakeOpenAPISchema("name", schema, schemaPath, schemaPackage, astmodel.NewIdentifierFactory(), loader) + wrappedSchema := MakeOpenAPISchema( + "name", + schema, + schemaPath, + schemaPackage, + astmodel.NewIdentifierFactory(), + loader, + logr.Discard()) typeName, err := wrappedSchema.refTypeName() g.Expect(err).ToNot(HaveOccurred()) @@ -179,7 +201,14 @@ func Test_GeneratingCollidingTypeNamesReturnsError(t *testing.T) { }, } - wrappedSchema := MakeOpenAPISchema("name", schema, schemaPath, schemaPackage, astmodel.NewIdentifierFactory(), loader) + wrappedSchema := MakeOpenAPISchema( + "name", + schema, + schemaPath, + schemaPackage, + astmodel.NewIdentifierFactory(), + loader, + logr.Discard()) _, err := wrappedSchema.refTypeName() g.Expect(err).To(HaveOccurred()) @@ -241,7 +270,14 @@ func Test_GeneratingCollidingTypeNamesWithSiblingFilesReturnsError(t *testing.T) }, } - wrappedSchema := MakeOpenAPISchema("name", schema, schemaPath, schemaPackage, astmodel.NewIdentifierFactory(), loader) + wrappedSchema := MakeOpenAPISchema( + "name", + schema, + schemaPath, + schemaPackage, + astmodel.NewIdentifierFactory(), + loader, + logr.Discard()) _, err := wrappedSchema.refTypeName() g.Expect(err).To(HaveOccurred()) diff --git a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go index cdd812fcfd6..834b35be1a5 100644 --- a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go +++ b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go @@ -11,10 +11,10 @@ import ( "regexp" "strings" + "github.com/go-logr/logr" "github.com/go-openapi/spec" "github.com/pkg/errors" "golang.org/x/exp/slices" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -22,13 +22,13 @@ import ( ) type SwaggerTypeExtractor struct { - idFactory astmodel.IdentifierFactory - config *config.Configuration - cache CachingFileLoader - swagger spec.Swagger - swaggerPath string - // package for output types (e.g. Microsoft.Network.Frontdoor/v20200101) - outputPackage astmodel.LocalPackageReference + idFactory astmodel.IdentifierFactory + config *config.Configuration + cache CachingFileLoader + swagger spec.Swagger + swaggerPath string + outputPackage astmodel.LocalPackageReference // package for output types (e.g. Microsoft.Network.Frontdoor/v20200101) + log logr.Logger } // NewSwaggerTypeExtractor creates a new SwaggerTypeExtractor @@ -39,6 +39,7 @@ func NewSwaggerTypeExtractor( swaggerPath string, outputPackage astmodel.LocalPackageReference, cache CachingFileLoader, + log logr.Logger, ) SwaggerTypeExtractor { return SwaggerTypeExtractor{ idFactory: idFactory, @@ -47,6 +48,7 @@ func NewSwaggerTypeExtractor( outputPackage: outputPackage, cache: cache, config: config, + log: log, } } @@ -120,7 +122,6 @@ func (extractor *SwaggerTypeExtractor) ExtractResourceTypes(ctx context.Context, specSchema, statusSchema, ok := extractor.findARMResourceSchema(op, rawOperationPath) if !ok { - // klog.Warningf("No ARM schema found for %s in %q", rawOperationPath, filePath) continue } @@ -160,13 +161,19 @@ func (extractor *SwaggerTypeExtractor) extractOneResourceType( armType, resourceName, err := extractor.resourceNameFromOperationPath(operationPath) if err != nil { - klog.Errorf("Error extracting resource name (%s): %s", extractor.swaggerPath, err.Error()) + extractor.log.V(1).Error( + err, + "Error extracting resource name", + "swaggerPath", extractor.swaggerPath) return nil } shouldPrune, because := scanner.configuration.ShouldPrune(resourceName) if shouldPrune == config.Prune { - klog.V(3).Infof("Skipping %s because %s", resourceName, because) + extractor.log.V(1).Info( + "Skipping resource", + "name", resourceName, + "because", because) return nil } @@ -236,8 +243,8 @@ func (extractor *SwaggerTypeExtractor) extractOneResourceType( } // ExtractOneOfTypes ensures we haven't missed any of the required OneOf type definitions. -// The depth-first search of the Swagger spec done by ExtractResourcetypes() won't have found any "loose" one of options -// so we need this extra step. +// The depth-first search of the Swagger spec done by ExtractResourcetypes() won't have found any "loose" one of +// options, so we need this extra step. func (extractor *SwaggerTypeExtractor) ExtractOneOfTypes( ctx context.Context, scanner *SchemaScanner, @@ -259,7 +266,8 @@ func (extractor *SwaggerTypeExtractor) ExtractOneOfTypes( extractor.swaggerPath, extractor.outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) // Run a handler to generate our type t, err := scanner.RunHandlerForSchema(ctx, schema) @@ -317,7 +325,10 @@ func (extractor *SwaggerTypeExtractor) findARMResourceSchema(op spec.PathItem, r params := op.Put.Parameters if op.Parameters != nil { - klog.Warningf("overriding parameters for %q in %s", rawOperationPath, extractor.swaggerPath) + extractor.log.V(1).Info( + "overriding parameters", + "operation", rawOperationPath, + "swagger", extractor.swaggerPath) params = op.Parameters } @@ -345,9 +356,14 @@ func (extractor *SwaggerTypeExtractor) findARMResourceSchema(op spec.PathItem, r if foundSpec == nil { if noBody { - klog.V(3).Infof("Empty body for %s", rawOperationPath) + extractor.log.V(2).Info( + "no body parameter found for PUT operation", + "operation", rawOperationPath) } else { - klog.Warningf("Response indicated that type was ARM resource but no schema found for %s in %q", rawOperationPath, extractor.swaggerPath) + extractor.log.Info( + "no schema found for PUT operation", + "operation", rawOperationPath, + "swagger", extractor.swaggerPath) return nil, nil, false } } @@ -377,7 +393,6 @@ func (extractor *SwaggerTypeExtractor) schemaFromParameter(param spec.Parameter) if param.Schema == nil { // We're dealing with a simple schema here if param.SimpleSchema.Type == "" { - klog.Warningf("schemaFromParameter invoked on parameter without schema or simpleschema") return nil } @@ -387,7 +402,8 @@ func (extractor *SwaggerTypeExtractor) schemaFromParameter(param spec.Parameter) paramPath, extractor.outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) } else { result = MakeOpenAPISchema( nameFromRef(param.Schema.Ref), @@ -395,7 +411,8 @@ func (extractor *SwaggerTypeExtractor) schemaFromParameter(param spec.Parameter) paramPath, extractor.outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) } return &result @@ -412,7 +429,8 @@ func (extractor *SwaggerTypeExtractor) doesResponseRepresentARMResource(response extractor.swaggerPath, extractor.outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) return &result, isMarkedAsARMResource(result) } @@ -432,12 +450,16 @@ func (extractor *SwaggerTypeExtractor) doesResponseRepresentARMResource(response refFilePath, outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) return &schema, isMarkedAsARMResource(schema) } - klog.Warningf("Unable to locate schema on response for %q in %s", rawOperationPath, extractor.swaggerPath) + extractor.log.Info( + "no schema found for response", + "operation", rawOperationPath, + "swagger", extractor.swaggerPath) return nil, false } @@ -582,7 +604,12 @@ func (extractor *SwaggerTypeExtractor) expandAndCanonicalizePath( if err != nil { // This error is safe to ignore in this case as it means that we can't parse the path as a resource. // Just return the raw path so we do further processing and log a message - klog.Errorf("Error expanding enums in path (%s): %s", extractor.swaggerPath, err.Error()) + // We don't log using Error() here because this is a common case and we don't want to spam the logs + // (Error logs are always emitted, and can't be suppressed) + extractor.log.V(1).Info( + "expanding enums in path", + "swaggerPath", extractor.swaggerPath, + "error", err.Error()) return results } From 65ca93532b75fb1a610bc66f9ad88aa099ee4c81 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 21 Apr 2023 14:18:41 +1200 Subject: [PATCH 04/21] Update Swagger parsing --- .../internal/codegen/golden_files_test.go | 3 +- .../internal/codegen/pipeline/load_types.go | 23 +++-- .../generator/internal/jsonast/jsonast.go | 85 ++++++++++++------- .../jsonast/swagger_type_extractor.go | 9 +- .../jsonast/swagger_type_extractor_test.go | 9 +- 5 files changed, 85 insertions(+), 44 deletions(-) diff --git a/v2/tools/generator/internal/codegen/golden_files_test.go b/v2/tools/generator/internal/codegen/golden_files_test.go index 9c5be7f799e..35a5f9dad77 100644 --- a/v2/tools/generator/internal/codegen/golden_files_test.go +++ b/v2/tools/generator/internal/codegen/golden_files_test.go @@ -15,6 +15,7 @@ import ( "strings" "testing" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/sebdah/goldie/v2" "github.com/xeipuuv/gojsonschema" @@ -241,7 +242,7 @@ func loadTestSchemaIntoTypes( return nil, errors.Wrapf(err, "could not compile input") } - scanner := jsonast.NewSchemaScanner(idFactory, configuration) + scanner := jsonast.NewSchemaScanner(idFactory, configuration, logr.Discard()) klog.V(0).Infof("Walking deployment template") diff --git a/v2/tools/generator/internal/codegen/pipeline/load_types.go b/v2/tools/generator/internal/codegen/pipeline/load_types.go index a7b196cb37c..607c1083fbe 100644 --- a/v2/tools/generator/internal/codegen/pipeline/load_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/load_types.go @@ -21,7 +21,6 @@ import ( "github.com/pkg/errors" "golang.org/x/sync/errgroup" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -47,14 +46,19 @@ func LoadTypes( LoadTypesStageID, "Load all types from Swagger files", func(ctx context.Context, definitions astmodel.TypeDefinitionSet) (astmodel.TypeDefinitionSet, error) { - klog.V(1).Infof("Loading Swagger data from %q", config.SchemaRoot) + log.V(1).Info( + "Loading Swagger data", + "source", config.SchemaRoot) swaggerTypes, err := loadSwaggerData(ctx, idFactory, config, log) if err != nil { return nil, errors.Wrapf(err, "unable to load Swagger data") } - klog.V(1).Infof("Loaded Swagger data (%d resources, %d other definitions)", len(swaggerTypes.ResourceDefinitions), len(swaggerTypes.OtherDefinitions)) + log.V(1).Info( + "Loaded Swagger data", + "resources", len(swaggerTypes.ResourceDefinitions), + "otherDefinitions", len(swaggerTypes.OtherDefinitions)) if len(swaggerTypes.ResourceDefinitions) == 0 || len(swaggerTypes.OtherDefinitions) == 0 { return nil, errors.Errorf("Failed to load swagger information") @@ -303,7 +307,13 @@ func loadSwaggerData( config *config.Configuration, log logr.Logger, ) (jsonast.SwaggerTypes, error) { - schemas, err := loadAllSchemas(ctx, config.SchemaRoot, config.LocalPathPrefix(), idFactory, config.Status.Overrides) + schemas, err := loadAllSchemas( + ctx, + config.SchemaRoot, + config.LocalPathPrefix(), + idFactory, + config.Status.Overrides, + log) if err != nil { return jsonast.SwaggerTypes{}, err } @@ -636,6 +646,7 @@ func loadAllSchemas( localPathPrefix string, idFactory astmodel.IdentifierFactory, overrides []config.SchemaOverride, + log logr.Logger, ) (map[string]jsonast.PackageAndSwagger, error) { var mutex sync.Mutex schemas := make(map[string]jsonast.PackageAndSwagger) @@ -673,7 +684,9 @@ func loadAllSchemas( eg.Go(func() error { var swagger spec.Swagger - klog.V(3).Infof("Loading file %q", filePath) + log.V(1).Info( + "Loading OpenAPI spec", + "file", filePath) fileContent, err := ioutil.ReadFile(filePath) if err != nil { diff --git a/v2/tools/generator/internal/jsonast/jsonast.go b/v2/tools/generator/internal/jsonast/jsonast.go index f415e3b5529..1a811512303 100644 --- a/v2/tools/generator/internal/jsonast/jsonast.go +++ b/v2/tools/generator/internal/jsonast/jsonast.go @@ -15,9 +15,9 @@ import ( "strings" "github.com/devigned/tab" + "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -29,7 +29,7 @@ type ( // TypeHandler is a standard delegate used for walking the schema tree. // Note that it is permissible for a TypeHandler to return `nil, nil`, which indicates that // there is no type to be included in the output. - TypeHandler func(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) + TypeHandler func(ctx context.Context, scanner *SchemaScanner, schema Schema, log logr.Logger) (astmodel.Type, error) // UnknownSchemaError is used when we find a JSON schema node that we don't know how to handle UnknownSchemaError struct { @@ -43,6 +43,7 @@ type ( TypeHandlers map[SchemaType]TypeHandler configuration *config.Configuration idFactory astmodel.IdentifierFactory + log logr.Logger } ) @@ -80,12 +81,17 @@ func (use *UnknownSchemaError) Error() string { } // NewSchemaScanner constructs a new scanner, ready for use -func NewSchemaScanner(idFactory astmodel.IdentifierFactory, configuration *config.Configuration) *SchemaScanner { +func NewSchemaScanner( + idFactory astmodel.IdentifierFactory, + configuration *config.Configuration, + log logr.Logger, +) *SchemaScanner { return &SchemaScanner{ definitions: make(map[astmodel.TypeName]*astmodel.TypeDefinition), TypeHandlers: defaultTypeHandlers(), configuration: configuration, idFactory: idFactory, + log: log, } } @@ -102,7 +108,7 @@ func (scanner *SchemaScanner) RunHandler(ctx context.Context, schemaType SchemaT } handler := scanner.TypeHandlers[schemaType] - return handler(ctx, scanner, schema) + return handler(ctx, scanner, schema, scanner.log) } // RunHandlerForSchema inspects the passed schema to identify what kind it is, then runs the appropriate handler @@ -126,7 +132,10 @@ func (scanner *SchemaScanner) RunHandlersForSchemas(ctx context.Context, schemas if errors.As(err, &unknownSchema) { if unknownSchema.Schema.description() != nil { // some Swagger types (e.g. ServiceFabric Cluster) use allOf with a description-only schema - klog.V(2).Infof("skipping description-only schema type with description %q", *unknownSchema.Schema.description()) + scanner.log.V(2).Info( + "skipping description-only schema type", + "schema", unknownSchema.Schema.url(), + "description", *unknownSchema.Schema.description()) continue } } @@ -212,7 +221,7 @@ func defaultTypeHandlers() map[SchemaType]TypeHandler { } } -func stringHandler(_ context.Context, _ *SchemaScanner, schema Schema) (astmodel.Type, error) { +func stringHandler(_ context.Context, _ *SchemaScanner, schema Schema, log logr.Logger) (astmodel.Type, error) { t := astmodel.StringType maxLength := schema.maxLength() @@ -221,13 +230,13 @@ func stringHandler(_ context.Context, _ *SchemaScanner, schema Schema) (astmodel format := schema.format() if maxLength != nil || minLength != nil || pattern != nil || format != "" { - patterns := []*regexp.Regexp{} + patterns := make([]*regexp.Regexp, 0, 2) if pattern != nil { patterns = append(patterns, pattern) } if format != "" { - formatPattern := formatToPattern(format) + formatPattern := formatToPattern(format, log) if formatPattern != nil { patterns = append(patterns, formatPattern) } @@ -258,7 +267,7 @@ func stringHandler(_ context.Context, _ *SchemaScanner, schema Schema) (astmodel // copied from ARM implementation var uuidRegex = regexp.MustCompile("^[0-9a-fA-F]{8}(-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}$") -func formatToPattern(format string) *regexp.Regexp { +func formatToPattern(format string, log logr.Logger) *regexp.Regexp { switch format { case "uuid": return uuidRegex @@ -270,12 +279,14 @@ func formatToPattern(format string) *regexp.Regexp { // ignore it for now return nil default: - klog.Warningf("unknown format %q", format) + log.V(1).Info( + "Unknown property format", + "format", format) return nil } } -func numberHandler(_ context.Context, _ *SchemaScanner, schema Schema) (astmodel.Type, error) { +func numberHandler(_ context.Context, _ *SchemaScanner, schema Schema, _ logr.Logger) (astmodel.Type, error) { t := astmodel.FloatType v := getNumberValidations(schema) if v != nil { @@ -318,7 +329,7 @@ var ( maxUint32 *big.Rat = big.NewRat(1, 1).SetUint64(math.MaxUint32) ) -func intHandler(_ context.Context, _ *SchemaScanner, schema Schema) (astmodel.Type, error) { +func intHandler(_ context.Context, _ *SchemaScanner, schema Schema, _ logr.Logger) (astmodel.Type, error) { t := astmodel.IntType v := getNumberValidations(schema) if v != nil { @@ -358,11 +369,11 @@ func getNumberValidations(schema Schema) *astmodel.NumberValidations { return nil } -func boolHandler(_ context.Context, _ *SchemaScanner, _ Schema) (astmodel.Type, error) { +func boolHandler(_ context.Context, _ *SchemaScanner, _ Schema, _ logr.Logger) (astmodel.Type, error) { return astmodel.BoolType, nil } -func enumHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func enumHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, _ logr.Logger) (astmodel.Type, error) { _, span := tab.StartSpan(ctx, "enumHandler") defer span.End() @@ -402,11 +413,11 @@ func enumHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (as return enumType, nil } -func objectHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func objectHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, log logr.Logger) (astmodel.Type, error) { ctx, span := tab.StartSpan(ctx, "objectHandler") defer span.End() - properties, err := getProperties(ctx, scanner, schema) + properties, err := getProperties(ctx, scanner, schema, log) if err != nil { return nil, err } @@ -423,7 +434,7 @@ func objectHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) ( // If we're a resource, our 'Id' property needs to have a special type if isResource { for i, prop := range properties { - if prop.HasName(astmodel.PropertyName("Id")) || prop.HasName(astmodel.PropertyName("ID")) { + if prop.HasName("Id") || prop.HasName("ID") { properties[i] = prop.WithType(astmodel.NewOptionalType(astmodel.ARMIDType)) } } @@ -474,6 +485,7 @@ func getProperties( ctx context.Context, scanner *SchemaScanner, schema Schema, + log logr.Logger, ) ([]*astmodel.PropertyDefinition, error) { ctx, span := tab.StartSpan(ctx, "getProperties") defer span.End() @@ -494,7 +506,9 @@ func getProperties( if property == nil { // TODO: This log shouldn't happen in cases where the type in question is later excluded, see: // TODO: https://github.com/Azure/azure-service-operator/issues/1517 - klog.V(2).Infof("Property %s omitted due to nil propType (probably due to type filter)", propName) + log.V(2).Info( + "Property omitted due to nil propType (probably due to type filter)", + "property", propName) continue } @@ -586,7 +600,7 @@ func getProperties( return properties, nil } -func refHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func refHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, log logr.Logger) (astmodel.Type, error) { ctx, span := tab.StartSpan(ctx, "refHandler") defer span.End() @@ -606,14 +620,21 @@ func refHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (ast // Prune the graph according to the configuration shouldPrune, because := scanner.configuration.ShouldPrune(typeName) if shouldPrune == config.Prune { - klog.V(3).Infof("Skipping %s because %s", typeName, because) + log.V(2).Info( + "Skipping type", + "type", typeName, + "because", because) return nil, nil // Skip entirely } // Target types according to configuration transformation, because := scanner.configuration.TransformType(typeName) if transformation != nil { - klog.V(2).Infof("Transforming %s -> %s because %s", typeName, transformation, because) + log.V(2).Info( + "Transforming type", + "type", typeName, + "because", because, + "transformation", transformation) return transformation, nil } @@ -631,7 +652,7 @@ func generateDefinitionsFor( return astmodel.EmptyTypeName, err } - url := schema.url() + schemaUrl := schema.url() // see if we already generated something for this ref if _, ok := scanner.findTypeDefinition(typeName); ok { @@ -650,7 +671,7 @@ func generateDefinitionsFor( // TODO: This code and below does nothing in the Swagger path as schema.url() is always empty. // TODO: It's still used in the JSON schema path for golden tests and should be removed once those // TODO: are retired. - resourceType := categorizeResourceType(url) + resourceType := categorizeResourceType(schemaUrl) if resourceType != nil { result = astmodel.NewAzureResourceType(result, nil, typeName, *resourceType) } @@ -681,7 +702,7 @@ func generateDefinitionsFor( return definition.Name(), nil } -func allOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func allOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, _ logr.Logger) (astmodel.Type, error) { ctx, span := tab.StartSpan(ctx, "allOfHandler") defer span.End() @@ -703,7 +724,7 @@ func allOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (a return astmodel.BuildAllOfType(types...), nil } -func oneOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func oneOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, _ logr.Logger) (astmodel.Type, error) { ctx, span := tab.StartSpan(ctx, "oneOfHandler") defer span.End() @@ -785,16 +806,18 @@ func asCommonProperties( return nil, false } -func anyOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func anyOfHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, log logr.Logger) (astmodel.Type, error) { ctx, span := tab.StartSpan(ctx, "anyOfHandler") defer span.End() // See https://github.com/Azure/azure-service-operator/issues/1518 for details about why this is treated as oneOf - klog.V(2).Infof("Handling anyOf type as if it were oneOf: %s\n", schema.url()) // TODO: was Ref.URL - return oneOfHandler(ctx, scanner, schema) + log.V(1).Info( + "Handling anyOf type as if it were oneOf", + "url", schema.url()) + return oneOfHandler(ctx, scanner, schema, log) } -func arrayHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (astmodel.Type, error) { +func arrayHandler(ctx context.Context, scanner *SchemaScanner, schema Schema, log logr.Logger) (astmodel.Type, error) { ctx, span := tab.StartSpan(ctx, "arrayHandler") defer span.End() @@ -805,7 +828,9 @@ func arrayHandler(ctx context.Context, scanner *SchemaScanner, schema Schema) (a if len(items) == 0 { // there is no type to the elements, so we must assume interface{} - klog.Warningf("Interface assumption unproven for %s\n", schema.url()) + log.V(1).Info( + "Interface assumption unproven", + "url", schema.url()) result := astmodel.NewArrayType(astmodel.AnyType) return withArrayValidations(schema, result), nil diff --git a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go index 834b35be1a5..263893fe4f8 100644 --- a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go +++ b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go @@ -78,7 +78,7 @@ func (extractor *SwaggerTypeExtractor) ExtractTypes(ctx context.Context) (Swagge OtherDefinitions: make(astmodel.TypeDefinitionSet), } - scanner := NewSchemaScanner(extractor.idFactory, extractor.config) + scanner := NewSchemaScanner(extractor.idFactory, extractor.config, extractor.log) err := extractor.ExtractResourceTypes(ctx, scanner, result) if err != nil { @@ -161,10 +161,11 @@ func (extractor *SwaggerTypeExtractor) extractOneResourceType( armType, resourceName, err := extractor.resourceNameFromOperationPath(operationPath) if err != nil { - extractor.log.V(1).Error( - err, + // Logging using Info() because this is common and Error() can't be suppressed + extractor.log.V(1).Info( "Error extracting resource name", - "swaggerPath", extractor.swaggerPath) + "swaggerPath", extractor.swaggerPath, + "error", err) return nil } diff --git a/v2/tools/generator/internal/jsonast/swagger_type_extractor_test.go b/v2/tools/generator/internal/jsonast/swagger_type_extractor_test.go index 633c4470f65..a2e04eb773d 100644 --- a/v2/tools/generator/internal/jsonast/swagger_type_extractor_test.go +++ b/v2/tools/generator/internal/jsonast/swagger_type_extractor_test.go @@ -9,6 +9,7 @@ import ( "context" "testing" + "github.com/go-logr/logr" "github.com/go-openapi/spec" . "github.com/onsi/gomega" @@ -222,7 +223,7 @@ func Test_ExpandAndCanonicalizePath_DoesNotExpandSimplePath(t *testing.T) { extractor := &SwaggerTypeExtractor{ idFactory: astmodel.NewIdentifierFactory(), } - scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration()) + scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration(), logr.Discard()) parameters := []spec.Parameter{ makeSubscriptionIDParameter(), @@ -257,7 +258,7 @@ func Test_ExpandAndCanonicalizePath_ExpandsSingleValueEnumInNameLocationWithDefa extractor := &SwaggerTypeExtractor{ idFactory: astmodel.NewIdentifierFactory(), } - scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration()) + scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration(), logr.Discard()) parameters := []spec.Parameter{ makeSubscriptionIDParameter(), @@ -295,7 +296,7 @@ func Test_ExpandAndCanonicalizePath_DoesNotExpandSingleValueEnumWithoutDefault(t extractor := &SwaggerTypeExtractor{ idFactory: astmodel.NewIdentifierFactory(), } - scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration()) + scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration(), logr.Discard()) parameters := []spec.Parameter{ makeSubscriptionIDParameter(), @@ -333,7 +334,7 @@ func Test_ExpandAndCanonicalizePath_ExpandsEnumInResourceTypePath(t *testing.T) extractor := &SwaggerTypeExtractor{ idFactory: astmodel.NewIdentifierFactory(), } - scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration()) + scanner := NewSchemaScanner(astmodel.NewIdentifierFactory(), config.NewConfiguration(), logr.Discard()) parameters := []spec.Parameter{ makeSubscriptionIDParameter(), From 5baf342d47afcf545da6beebb02c2979674d1051 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 11:16:41 +1200 Subject: [PATCH 05/21] Catch panics during stage execution --- .../internal/codegen/code_generator.go | 76 ++++++++++++++----- 1 file changed, 56 insertions(+), 20 deletions(-) diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index 3da22c09f3d..deaedf38c07 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -281,23 +281,15 @@ func (generator *CodeGenerator) Generate( state := pipeline.NewState() for i, stage := range generator.pipeline { - stageDescription := fmt.Sprintf("%d/%d: %s", i+1, len(generator.pipeline), stage.Description()) + stageNumber := i + 1 + stageDescription := fmt.Sprintf("%d/%d: %s", stageNumber, len(generator.pipeline), stage.Description()) log.Info(stageDescription) start := time.Now() - newState, err := stage.Run(ctx, state) + newState, err := generator.executeStage(ctx, stageNumber, stage, state) if err != nil { - return errors.Wrapf(err, "failed during pipeline stage %d/%d [%s]: %s", i+1, len(generator.pipeline), stage.Id(), stage.Description()) - } - - if ctx.Err() != nil { - // Cancelled - return errors.Wrapf(ctx.Err(), "pipeline cancelled during stage %d/%d [%s]: %s", i+1, len(generator.pipeline), stage.Id(), stage.Description()) - } - - // Fail fast if something goes awry - if len(newState.Definitions()) == 0 { - return errors.Errorf("all type definitions removed by stage %s", stage.Id()) + log.Error(err, "failed to execute stage", "stage", stage.Description()) + return errors.Wrapf(err, "failed to execute stage %d: %s", stageNumber, stage.Description()) } duration := time.Since(start).Round(time.Millisecond) @@ -323,13 +315,6 @@ func (generator *CodeGenerator) Generate( "removed", len(defsRemoved)) } - if generator.debugReporter != nil { - err := generator.debugReporter.ReportStage(i, stage.Description(), newState) - if err != nil { - return errors.Wrapf(err, "failed to generate debug report for stage %d/%d: %s", i+1, len(generator.pipeline), stage.Description()) - } - } - state = newState } @@ -343,6 +328,57 @@ func (generator *CodeGenerator) Generate( return nil } +// executeStage runs the given stage against the given state, returning the new state. +// Any error generated will be wrapped with details of the stage by our caller. +// ctx is used to cancel the generation process. +// stage is the stage to execute. +// state is the state to execute the stage against. +func (generator *CodeGenerator) executeStage( + ctx context.Context, + stageNumber int, + stage *pipeline.Stage, + state *pipeline.State, +) (newState *pipeline.State, err error) { + // Catch any panics and turn them into errors, unless they already are + defer func() { + if r := recover(); r != nil { + if e, ok := r.(error); ok { + err = e + } else { + err = errors.Errorf("panic: %v", r) + } + } + }() + + // Run the stage + newState, err = stage.Run(ctx, state) + if err != nil { + // Will be wrapped by our caller with details of the stage + return nil, err + } + + if ctx.Err() != nil { + // Cancelled - will be wrapped by our caller + return nil, errors.New("pipeline cancelled") + } + + // Fail fast if something goes awry + if len(newState.Definitions()) == 0 { + // Will be wrapped by our caller with details of the stage + return nil, errors.Errorf("all type definitions removed by stage") + } + + if generator.debugReporter != nil { + err := generator.debugReporter.ReportStage(stageNumber, stage.Description(), newState) + if err != nil { + // Will be wrapped by our caller with details of the stage + return nil, errors.Wrapf(err, "failed to generate debug report for stage") + } + } + + return +} + // RemoveStages will remove all stages from the pipeline with the given ids. // Only available for test builds. // Will panic if you specify an unknown id. From 822c0fa6a28cbf30a3c80e71d6e1fe807da6efaa Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 12:12:51 +1200 Subject: [PATCH 06/21] Remove logging we don't need --- .../astmodel/kubebuilder_validations.go | 19 +++---------------- .../internal/astmodel/package_definition.go | 3 --- .../internal/astmodel/resource_type.go | 3 --- .../codegen/embeddedresources/remover.go | 4 ---- .../codegen/embeddedresources/renamer.go | 3 --- .../internal/codegen/golden_files_test.go | 5 ----- .../codegen/pipeline/assemble_oneof.go | 5 ----- .../codegen/pipeline/check_for_anytype.go | 10 +--------- .../codegen/pipeline/flatten_properties.go | 6 ------ .../inject_property_assignment_functions.go | 4 ---- .../inject_spec_initialization_functions.go | 3 --- .../internal/codegen/pipeline/load_types.go | 9 --------- .../make_status_properties_optional.go | 5 ----- .../codegen/pipeline/pipeline_diagram.go | 2 -- .../simple_recursive_type_fixer.go | 4 +--- .../codegen/pipeline/remove_type_aliases.go | 2 -- .../pipeline/report_resource_versions.go | 9 +-------- .../codegen/pipeline/simplify_definitions.go | 4 ---- .../internal/codegen/pipeline/stage.go | 12 +----------- .../internal/jsonast/openapi_file_loader.go | 3 --- .../testcases/json_serialization_test_case.go | 14 -------------- 21 files changed, 7 insertions(+), 122 deletions(-) diff --git a/v2/tools/generator/internal/astmodel/kubebuilder_validations.go b/v2/tools/generator/internal/astmodel/kubebuilder_validations.go index d4712e997c3..c53fd55c28d 100644 --- a/v2/tools/generator/internal/astmodel/kubebuilder_validations.go +++ b/v2/tools/generator/internal/astmodel/kubebuilder_validations.go @@ -13,7 +13,6 @@ import ( "strings" "github.com/dave/dst" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" ) @@ -158,11 +157,7 @@ func MakeMaximumValidation(value *big.Rat) KubeBuilderValidation { if value.IsInt() { return KubeBuilderValidation{MaximumValidationName, value.RatString()} } else { - floatValue, ok := value.Float64() - if !ok { - klog.Warningf("inexact maximum: %s ⇒ %g", value.String(), floatValue) - } - + floatValue, _ := value.Float64() return KubeBuilderValidation{MaximumValidationName, floatValue} } } @@ -171,11 +166,7 @@ func MaxMinimumValidation(value *big.Rat) KubeBuilderValidation { if value.IsInt() { return KubeBuilderValidation{MinimumValidationName, value.RatString()} } else { - floatValue, ok := value.Float64() - if !ok { - klog.Warningf("inexact minimum: %s ⇒ %g", value.String(), floatValue) - } - + floatValue, _ := value.Float64() return KubeBuilderValidation{MinimumValidationName, floatValue} } } @@ -192,11 +183,7 @@ func MakeMultipleOfValidation(value *big.Rat) KubeBuilderValidation { if value.IsInt() { return KubeBuilderValidation{MultipleOfValidationName, value.RatString()} } else { - floatValue, ok := value.Float64() - if !ok { - klog.Warningf("inexact multiple-of: %s ⇒ %g", value.String(), floatValue) - } - + floatValue, _ := value.Float64() return KubeBuilderValidation{MultipleOfValidationName, floatValue} } } diff --git a/v2/tools/generator/internal/astmodel/package_definition.go b/v2/tools/generator/internal/astmodel/package_definition.go index 914fbc6b1be..f8c425a26a9 100644 --- a/v2/tools/generator/internal/astmodel/package_definition.go +++ b/v2/tools/generator/internal/astmodel/package_definition.go @@ -16,7 +16,6 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "github.com/pkg/errors" - "k8s.io/klog/v2" ) // PackageDefinition is the definition of a package @@ -122,7 +121,6 @@ func (p *PackageDefinition) writeCodeFile( ref := defs[0].Name().PackageReference genFile := NewFileDefinition(ref, defs, packages) - klog.V(5).Infof("Writing code file %q\n", outputFile) fileWriter := NewGoSourceFileWriter(genFile) err := fileWriter.SaveToFile(outputFile) if err != nil { @@ -154,7 +152,6 @@ func (p *PackageDefinition) writeTestFile( ref := defs[0].Name().PackageReference genFile := NewTestFileDefinition(ref, defs, packages) - klog.V(5).Infof("Writing test case file %q\n", outputFile) fileWriter := NewGoSourceFileWriter(genFile) err := fileWriter.SaveToFile(outputFile) if err != nil { diff --git a/v2/tools/generator/internal/astmodel/resource_type.go b/v2/tools/generator/internal/astmodel/resource_type.go index 422ea2793be..e2d6c860082 100644 --- a/v2/tools/generator/internal/astmodel/resource_type.go +++ b/v2/tools/generator/internal/astmodel/resource_type.go @@ -13,7 +13,6 @@ import ( "github.com/dave/dst" "golang.org/x/exp/maps" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" ) @@ -111,8 +110,6 @@ func NewAzureResourceType(specType Type, statusType Type, typeName TypeName, sco } if nameProperty == nil { - klog.V(1).Infof("resource %s is missing field 'Name', fabricating one...", typeName) - nameProperty = NewPropertyDefinition("Name", "name", StringType) nameProperty.WithDescription("The name of the resource") isNameOptional = true diff --git a/v2/tools/generator/internal/codegen/embeddedresources/remover.go b/v2/tools/generator/internal/codegen/embeddedresources/remover.go index b700e92c244..a8cd784364d 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/remover.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/remover.go @@ -11,7 +11,6 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -245,8 +244,6 @@ func (e EmbeddedResourceRemover) newResourceRemovalTypeWalker(visitor astmodel.T typedCtx.modifiedDefinitions.Add(updated) } - klog.V(5).Infof("Updating %q to %q", original.Name(), updated.Name()) - return updated, nil } @@ -270,7 +267,6 @@ func (e EmbeddedResourceRemover) newResourceRemovalTypeWalker(visitor astmodel.T // Sometimes these resource-like things are promoted to real resources in future APIs as in the case of Subnet in the 2017-06-01 // API version. if isTypeResourceLookalike(def.Type()) { - klog.V(5).Infof("Type %q is a resource lookalike", def.Name()) return true, nil } diff --git a/v2/tools/generator/internal/codegen/embeddedresources/renamer.go b/v2/tools/generator/internal/codegen/embeddedresources/renamer.go index 61fc3a633a6..28c6a409ffb 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/renamer.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/renamer.go @@ -9,7 +9,6 @@ import ( "fmt" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -34,7 +33,6 @@ func (r renamer) simplifyEmbeddedNameToOriginalName( return nil, nil } - klog.V(4).Infof("There are no usages of %q. Collapsing %q into the original for simplicity.", original, associatedNames.Single()) renames := make(astmodel.TypeAssociation) renames[associatedNames.Single()] = original @@ -61,7 +59,6 @@ func (r renamer) simplifyEmbeddedNameRemoveContextAndCount( embeddedName.context = "" embeddedName.count = 0 renames[associated] = embeddedName.ToSimplifiedTypeName() - klog.V(4).Infof("There is only a single context %q is used in. Renaming it to %q for simplicity.", associated, renames[associated]) return renames, nil } diff --git a/v2/tools/generator/internal/codegen/golden_files_test.go b/v2/tools/generator/internal/codegen/golden_files_test.go index 35a5f9dad77..53e1c5f262c 100644 --- a/v2/tools/generator/internal/codegen/golden_files_test.go +++ b/v2/tools/generator/internal/codegen/golden_files_test.go @@ -20,7 +20,6 @@ import ( "github.com/sebdah/goldie/v2" "github.com/xeipuuv/gojsonschema" "gopkg.in/yaml.v3" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/codegen/pipeline" @@ -229,8 +228,6 @@ func loadTestSchemaIntoTypes( "loadTestSchema", "Load and walk schema (test)", func(ctx context.Context, state *pipeline.State) (*pipeline.State, error) { - klog.V(0).Infof("Loading test schema from %q", path) - inputFile, err := ioutil.ReadFile(path) if err != nil { return nil, errors.Wrapf(err, "cannot read golden test input file") @@ -244,8 +241,6 @@ func loadTestSchemaIntoTypes( scanner := jsonast.NewSchemaScanner(idFactory, configuration, logr.Discard()) - klog.V(0).Infof("Walking deployment template") - schemaAbstraction := jsonast.MakeGoJSONSchema(schema.Root(), configuration.MakeLocalPackageReference, idFactory) _, err = scanner.GenerateAllDefinitions(ctx, schemaAbstraction) if err != nil { diff --git a/v2/tools/generator/internal/codegen/pipeline/assemble_oneof.go b/v2/tools/generator/internal/codegen/pipeline/assemble_oneof.go index 16d235db609..0459cf5e5e3 100644 --- a/v2/tools/generator/internal/codegen/pipeline/assemble_oneof.go +++ b/v2/tools/generator/internal/codegen/pipeline/assemble_oneof.go @@ -11,7 +11,6 @@ import ( "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -322,10 +321,6 @@ func (oa *oneOfAssembler) addDiscriminatorProperty(name astmodel.TypeName, rootN propertyJson, astmodel.NewOptionalType(enumType)) - if discriminatorValue == "" { - klog.Warning("Weirdness") - } - obj := astmodel.NewObjectType().WithProperty(property) return oneOf.WithAdditionalPropertyObject(obj), nil }) diff --git a/v2/tools/generator/internal/codegen/pipeline/check_for_anytype.go b/v2/tools/generator/internal/codegen/pipeline/check_for_anytype.go index d2cc0840ba4..073de015ec1 100644 --- a/v2/tools/generator/internal/codegen/pipeline/check_for_anytype.go +++ b/v2/tools/generator/internal/codegen/pipeline/check_for_anytype.go @@ -10,10 +10,9 @@ import ( "sort" "strings" + "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/pkg/errors" - "k8s.io/klog/v2" - "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -126,13 +125,6 @@ func collectBadPackages( } sort.Strings(groupNames) - if klog.V(2).Enabled() { - for _, groupName := range groupNames { - sort.Strings(grouped[groupName]) - klog.Infof("%s: %s", groupName, grouped[groupName]) - } - } - // Complain if there were some packages where we expected problems // but didn't see any. if len(expectedPackages) > 0 { diff --git a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go index 0e6a14d4608..2412afb7af8 100644 --- a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go +++ b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -58,10 +57,6 @@ func makeFlatteningVisitor(defs astmodel.TypeDefinitionSet) astmodel.TypeVisitor // fix any colliding names: newProps = fixCollisions(newProps) - if len(newProps) != it.Properties().Len() { - klog.V(4).Infof("Flattened properties in %s", name) - } - result := it.WithoutProperties().WithProperties(newProps...) return result, nil @@ -156,7 +151,6 @@ func collectAndFlattenProperties( innerProps, err := flattenProperty(container, prop, defs) if err != nil { - klog.Warningf("Skipping flatten of %s on %s: %s", prop.PropertyName(), container, err) innerProps = []*astmodel.PropertyDefinition{ prop.SetFlatten(false), } diff --git a/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go b/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go index d6c6061a95e..faaec434909 100644 --- a/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go +++ b/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go @@ -10,7 +10,6 @@ import ( "github.com/dave/dst" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -41,12 +40,9 @@ func InjectPropertyAssignmentFunctions( _, ok := astmodel.AsFunctionContainer(def.Type()) if !ok { // just skip it - not a resource nor an object - klog.V(4).Infof("Skipping %s as no conversion functions needed", name) continue } - klog.V(3).Infof("Injecting conversion functions into %s", name) - // Find the definition we want to convert to/from nextName, err := state.ConversionGraph().FindNextType(name, state.Definitions()) if err != nil { diff --git a/v2/tools/generator/internal/codegen/pipeline/inject_spec_initialization_functions.go b/v2/tools/generator/internal/codegen/pipeline/inject_spec_initialization_functions.go index f75dbe26bfc..4e5c0e20f2b 100644 --- a/v2/tools/generator/internal/codegen/pipeline/inject_spec_initialization_functions.go +++ b/v2/tools/generator/internal/codegen/pipeline/inject_spec_initialization_functions.go @@ -10,7 +10,6 @@ import ( "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/codegen/storage" @@ -45,8 +44,6 @@ func InjectSpecInitializationFunctions( newDefs := make(astmodel.TypeDefinitionSet, len(mappings)) var errs []error for specName, statusName := range mappings { - klog.V(3).Infof("Injecting specName initialization function into %s", specName.String()) - spec := defs[specName] status := defs[statusName] diff --git a/v2/tools/generator/internal/codegen/pipeline/load_types.go b/v2/tools/generator/internal/codegen/pipeline/load_types.go index 607c1083fbe..122ae7b6010 100644 --- a/v2/tools/generator/internal/codegen/pipeline/load_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/load_types.go @@ -120,8 +120,6 @@ func LoadTypes( } } - klog.V(1).Infof("Input %d definitions, output %d definitions", len(definitions), len(defs)) - return defs, nil }) } @@ -344,15 +342,12 @@ func loadSwaggerData( } func mergeSwaggerTypesByGroup(idFactory astmodel.IdentifierFactory, m map[astmodel.LocalPackageReference][]typesFromFile) (jsonast.SwaggerTypes, error) { - klog.V(3).Infof("Merging types for %d groups/versions", len(m)) - result := jsonast.SwaggerTypes{ ResourceDefinitions: make(jsonast.ResourceDefinitionSet), OtherDefinitions: make(astmodel.TypeDefinitionSet), } for pkg, group := range m { - klog.V(3).Infof("Merging types for %s", pkg) merged := mergeTypesForPackage(idFactory, group) for rn, rt := range merged.ResourceDefinitions { if _, ok := result.ResourceDefinitions[rn]; ok { @@ -413,8 +408,6 @@ func mergeTypesForPackage(idFactory astmodel.IdentifierFactory, typesFromFiles [ names[ix] = newName.Name() renames[ttc.typesFromFileIx][name] = newName } - - klog.V(3).Infof("Conflicting definitions for %s, renaming to: %s", name, strings.Join(names, ", ")) } for ix := range typesFromFiles { @@ -732,14 +725,12 @@ func groupFromPath(filePath string, rootPath string, overrides []config.SchemaOv if strings.HasPrefix(filePath, configSchemaPath) { // a forced namespace: use it if schemaOverride.Namespace != "" { - klog.V(1).Infof("Overriding namespace to %s for file %s", schemaOverride.Namespace, filePath) return schemaOverride.Namespace } // found a suffix override: apply it if schemaOverride.Suffix != "" { group = group + "." + schemaOverride.Suffix - klog.V(1).Infof("Overriding namespace to %s for file %s", group, filePath) return group } } diff --git a/v2/tools/generator/internal/codegen/pipeline/make_status_properties_optional.go b/v2/tools/generator/internal/codegen/pipeline/make_status_properties_optional.go index 7124e00530a..137cb07ddf1 100644 --- a/v2/tools/generator/internal/codegen/pipeline/make_status_properties_optional.go +++ b/v2/tools/generator/internal/codegen/pipeline/make_status_properties_optional.go @@ -9,7 +9,6 @@ import ( "context" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -63,11 +62,7 @@ func makeStatusPropertiesOptional(statusDef astmodel.TypeDefinition) (astmodel.T // makeObjectPropertiesOptional makes properties optional for the object func makeObjectPropertiesOptional(this *astmodel.TypeVisitor, ot *astmodel.ObjectType, ctx interface{}) (astmodel.Type, error) { - typeName := ctx.(astmodel.TypeName) ot.Properties().ForEach(func(property *astmodel.PropertyDefinition) { - if property.IsRequired() { - klog.V(4).Infof("\"%s.%s\" was required, changing it to optional", typeName.String(), property.PropertyName()) - } ot = ot.WithProperty(property.MakeOptional().MakeTypeOptional()) }) diff --git a/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go b/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go index 9d20385ba57..3453e9bbad6 100644 --- a/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go +++ b/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go @@ -13,7 +13,6 @@ import ( "unicode" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" ) @@ -38,7 +37,6 @@ func (diagram *PipelineDiagram) WriteDiagram(stages []*Stage) error { dotsrc := diagram.createDiagram(stages) filename := filepath.Join(diagram.debugDir, "pipeline.dot") err := ioutil.WriteFile(filename, dotsrc, 0600) - klog.V(2).Infof("Wrote diagram for pipeline to %s", filename) return errors.Wrapf(err, "failed to write diagram to %s", filename) } diff --git a/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go b/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go index b6c331befea..9cdae9e0483 100644 --- a/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go +++ b/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go @@ -6,10 +6,9 @@ package recursivetypefixer import ( + "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/pkg/errors" - "k8s.io/klog/v2" - "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -102,7 +101,6 @@ func (s *SimpleRecursiveTypeFixer) unrollRecursiveReference(this *astmodel.TypeV return astmodel.IdentityVisitOfTypeName(this, it, ctx) } - klog.V(3).Infof("%q references itself, removing the self-reference by unrolling to %q", typedCtx.name, typedCtx.unrolledName) return astmodel.IdentityVisitOfTypeName(this, typedCtx.unrolledName, typedCtx) } diff --git a/v2/tools/generator/internal/codegen/pipeline/remove_type_aliases.go b/v2/tools/generator/internal/codegen/pipeline/remove_type_aliases.go index 20f8d53db80..7f306e89b21 100644 --- a/v2/tools/generator/internal/codegen/pipeline/remove_type_aliases.go +++ b/v2/tools/generator/internal/codegen/pipeline/remove_type_aliases.go @@ -12,7 +12,6 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -82,7 +81,6 @@ func resolveTypeName(visitor *astmodel.TypeVisitor, name astmodel.TypeName, defi return def.Name(), nil // must remain named case astmodel.TypeName: // We need to resolve further because this type is an alias - klog.V(3).Infof("Found type alias %s, replacing it with %s", name, concreteType) return resolveTypeName(visitor, concreteType, definitions) case *astmodel.PrimitiveType: return visitor.Visit(concreteType, nil) diff --git a/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go b/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go index b3ef544e7d0..28ab331da92 100644 --- a/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go +++ b/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go @@ -15,13 +15,11 @@ import ( "sort" "strings" + "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/pkg/errors" "golang.org/x/text/cases" "golang.org/x/text/language" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" - - "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -171,8 +169,6 @@ func (report *ResourceVersionsReport) summarize(definitions astmodel.TypeDefinit // SaveAllResourcesReportTo creates a file containing a report listing all supported resources // outputFile is the path to the file to create func (report *ResourceVersionsReport) SaveAllResourcesReportTo(outputFile string) error { - - klog.V(1).Infof("Writing report to %s", outputFile) frontMatter := report.readFrontMatter(outputFile) var buffer strings.Builder @@ -205,8 +201,6 @@ func (report *ResourceVersionsReport) ensureFolderExists(outputFile string) erro // group identifies the set of resources to include. // outputFile is the path to the file to create. func (report *ResourceVersionsReport) SaveGroupResourcesReportTo(group string, outputFile string) error { - - klog.V(1).Infof("Writing report to %s for group ", outputFile, group) frontMatter := report.readFrontMatter(outputFile) var buffer strings.Builder @@ -530,7 +524,6 @@ func (report *ResourceVersionsReport) readFrontMatter(outputPath string) string return "" } - klog.V(2).Infof("Reading front matter from %s", outputPath) data, err := os.ReadFile(outputPath) if err != nil { return "" diff --git a/v2/tools/generator/internal/codegen/pipeline/simplify_definitions.go b/v2/tools/generator/internal/codegen/pipeline/simplify_definitions.go index 9e868b48a3a..c44c2c6278f 100644 --- a/v2/tools/generator/internal/codegen/pipeline/simplify_definitions.go +++ b/v2/tools/generator/internal/codegen/pipeline/simplify_definitions.go @@ -9,7 +9,6 @@ import ( "context" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -31,9 +30,6 @@ func SimplifyDefinitions() *Stage { errs = append(errs, err) } else { result.Add(visited) - if !astmodel.TypeEquals(def.Type(), visited.Type()) { - klog.V(3).Infof("Simplified %s from %s to %s", def.Name(), def.Type(), visited.Type()) - } } } diff --git a/v2/tools/generator/internal/codegen/pipeline/stage.go b/v2/tools/generator/internal/codegen/pipeline/stage.go index 2bfaec4311a..6555b212836 100644 --- a/v2/tools/generator/internal/codegen/pipeline/stage.go +++ b/v2/tools/generator/internal/codegen/pipeline/stage.go @@ -10,11 +10,10 @@ import ( "fmt" "strings" + "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" - "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -207,8 +206,6 @@ func (stage *Stage) Run(ctx context.Context, state *State) (*State, error) { // checkPreconditions checks to ensure the preconditions of this stage have been satisfied func (stage *Stage) checkPreconditions(state *State) error { - klog.V(3).Infof("Checking requisites of %s", stage.Id()) - if err := stage.checkPrerequisites(state); err != nil { return err } @@ -225,12 +222,6 @@ func (stage *Stage) checkPrerequisites(state *State) error { var errs []error for _, prereq := range stage.prerequisites { satisfied := state.stagesSeen.Contains(prereq) - if satisfied { - klog.V(3).Infof("[✓] Required prerequisite %s satisfied", prereq) - } else { - klog.V(3).Infof("[✗] Required prerequisite %s NOT satisfied", prereq) - } - if !satisfied { errs = append(errs, errors.Errorf("prerequisite %q of stage %q NOT satisfied.", prereq, stage.Id())) } @@ -245,7 +236,6 @@ func (stage *Stage) checkPostrequisites(state *State) error { for _, postreq := range stage.postrequisites { early := state.stagesSeen.Contains(postreq) if early { - klog.V(3).Infof("[✗] Required postrequisite %s satisfied early", postreq) errs = append(errs, errors.Errorf("postrequisite %q satisfied of stage %q early.", postreq, stage.Id())) } } diff --git a/v2/tools/generator/internal/jsonast/openapi_file_loader.go b/v2/tools/generator/internal/jsonast/openapi_file_loader.go index f97a14e618c..5320e14ce84 100644 --- a/v2/tools/generator/internal/jsonast/openapi_file_loader.go +++ b/v2/tools/generator/internal/jsonast/openapi_file_loader.go @@ -12,7 +12,6 @@ import ( "github.com/go-openapi/spec" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -71,8 +70,6 @@ func (fileCache CachingFileLoader) loadFile(absPath string) (PackageAndSwagger, // which indicates to the caller to reuse the existing package for definitions result := PackageAndSwagger{} - klog.V(3).Infof("Loading file into cache %q", absPath) - fileContent, err := ioutil.ReadFile(absPath) if err != nil { return result, errors.Wrapf(err, "unable to read swagger file %q", absPath) diff --git a/v2/tools/generator/internal/testcases/json_serialization_test_case.go b/v2/tools/generator/internal/testcases/json_serialization_test_case.go index 54c74b04842..1d1446c07a0 100644 --- a/v2/tools/generator/internal/testcases/json_serialization_test_case.go +++ b/v2/tools/generator/internal/testcases/json_serialization_test_case.go @@ -12,7 +12,6 @@ import ( "github.com/dave/dst" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -76,7 +75,6 @@ func (o *JSONSerializationTestCase) AsFuncs(name astmodel.TypeName, genContext * if haveLargeObject { simpleGenerators = nil - klog.V(3).Infof("Skipping tests for primitive properties on large object %s", name) } // Find all the complex generators (dependent on other generators we'll be generating elsewhere) @@ -116,18 +114,6 @@ func (o *JSONSerializationTestCase) AsFuncs(name astmodel.TypeName, genContext * result = append(result, o.createGeneratorsFactoryMethod(o.idOfRelatedGeneratorsFactoryMethod(), relatedGenerators, genContext)) } - if len(errs) > 0 { - i := "issues" - if len(errs) == 1 { - i = "issue" - } - - klog.Warningf("Encountered %d %s creating JSON Serialisation test for %s", len(errs), i, name) - for _, err := range errs { - klog.Warning(err) - } - } - return result } From b8a6639ed89ff1fea0524c850c8a7a4c4eb493de Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 12:23:01 +1200 Subject: [PATCH 07/21] Fix deprecations --- v2/tools/generator/internal/codegen/golden_files_test.go | 5 ++--- v2/tools/generator/internal/codegen/pipeline/load_types.go | 3 +-- .../generator/internal/codegen/pipeline/pipeline_diagram.go | 4 ++-- v2/tools/generator/internal/jsonast/openapi_file_loader.go | 4 ++-- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/v2/tools/generator/internal/codegen/golden_files_test.go b/v2/tools/generator/internal/codegen/golden_files_test.go index 53e1c5f262c..cadf94cd018 100644 --- a/v2/tools/generator/internal/codegen/golden_files_test.go +++ b/v2/tools/generator/internal/codegen/golden_files_test.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "fmt" - "io/ioutil" "os" "path/filepath" "strings" @@ -47,7 +46,7 @@ func makeDefaultTestConfig() GoldenTestConfig { func loadTestConfig(path string) (GoldenTestConfig, error) { result := makeDefaultTestConfig() - fileBytes, err := ioutil.ReadFile(path) + fileBytes, err := os.ReadFile(path) if err != nil { // If the file doesn't exist we just use the default if os.IsNotExist(err) { @@ -228,7 +227,7 @@ func loadTestSchemaIntoTypes( "loadTestSchema", "Load and walk schema (test)", func(ctx context.Context, state *pipeline.State) (*pipeline.State, error) { - inputFile, err := ioutil.ReadFile(path) + inputFile, err := os.ReadFile(path) if err != nil { return nil, errors.Wrapf(err, "cannot read golden test input file") } diff --git a/v2/tools/generator/internal/codegen/pipeline/load_types.go b/v2/tools/generator/internal/codegen/pipeline/load_types.go index 122ae7b6010..fa7d68dab2f 100644 --- a/v2/tools/generator/internal/codegen/pipeline/load_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/load_types.go @@ -8,7 +8,6 @@ package pipeline import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "regexp" @@ -681,7 +680,7 @@ func loadAllSchemas( "Loading OpenAPI spec", "file", filePath) - fileContent, err := ioutil.ReadFile(filePath) + fileContent, err := os.ReadFile(filePath) if err != nil { return errors.Wrapf(err, "unable to read swagger file %q", filePath) } diff --git a/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go b/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go index 3453e9bbad6..0f05522c9ef 100644 --- a/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go +++ b/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go @@ -7,7 +7,7 @@ package pipeline import ( "fmt" - "io/ioutil" + "os" "path/filepath" "strings" "unicode" @@ -36,7 +36,7 @@ func NewPipelineDiagram(debugDir string) *PipelineDiagram { func (diagram *PipelineDiagram) WriteDiagram(stages []*Stage) error { dotsrc := diagram.createDiagram(stages) filename := filepath.Join(diagram.debugDir, "pipeline.dot") - err := ioutil.WriteFile(filename, dotsrc, 0600) + err := os.WriteFile(filename, dotsrc, 0600) return errors.Wrapf(err, "failed to write diagram to %s", filename) } diff --git a/v2/tools/generator/internal/jsonast/openapi_file_loader.go b/v2/tools/generator/internal/jsonast/openapi_file_loader.go index 5320e14ce84..eeb100062f3 100644 --- a/v2/tools/generator/internal/jsonast/openapi_file_loader.go +++ b/v2/tools/generator/internal/jsonast/openapi_file_loader.go @@ -7,7 +7,7 @@ package jsonast import ( "fmt" - "io/ioutil" + "os" "path/filepath" "github.com/go-openapi/spec" @@ -70,7 +70,7 @@ func (fileCache CachingFileLoader) loadFile(absPath string) (PackageAndSwagger, // which indicates to the caller to reuse the existing package for definitions result := PackageAndSwagger{} - fileContent, err := ioutil.ReadFile(absPath) + fileContent, err := os.ReadFile(absPath) if err != nil { return result, errors.Wrapf(err, "unable to read swagger file %q", absPath) } From 380d395c6b7edc0b7d086079742365d1ced9ba3f Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 12:45:48 +1200 Subject: [PATCH 08/21] Turn logs into panics --- .../generator/internal/astmodel/allof_type.go | 6 ++---- .../generator/internal/astmodel/enum_type.go | 12 +++++------ .../internal/astmodel/errored_type.go | 21 +++++++++++-------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/v2/tools/generator/internal/astmodel/allof_type.go b/v2/tools/generator/internal/astmodel/allof_type.go index 22c7a9e9b95..9731aee4113 100644 --- a/v2/tools/generator/internal/astmodel/allof_type.go +++ b/v2/tools/generator/internal/astmodel/allof_type.go @@ -12,7 +12,6 @@ import ( "github.com/dave/dst" "github.com/pkg/errors" - "k8s.io/klog/v2" ) // AllOfType represents something that is the union @@ -82,9 +81,8 @@ func BuildAllOfType(types ...Type) Type { return onlyOneOf.WithTypes(ts) } else if len(oneOfs) > 1 { - // emit a warning if this ever comes up - // (it doesn't at the moment) - klog.Warningf("More than one oneOf inside allOf") + // panic if this ever comes up (it doesn't at the moment) + panic(errors.New("More than one oneOf inside allOf")) } // 0 oneOf (nothing to do) or >1 oneOf (too hard) diff --git a/v2/tools/generator/internal/astmodel/enum_type.go b/v2/tools/generator/internal/astmodel/enum_type.go index c63746dce7b..07185c26b21 100644 --- a/v2/tools/generator/internal/astmodel/enum_type.go +++ b/v2/tools/generator/internal/astmodel/enum_type.go @@ -8,12 +8,13 @@ package astmodel import ( "fmt" "go/token" - "golang.org/x/exp/slices" "sort" "strings" + "github.com/pkg/errors" + "golang.org/x/exp/slices" + "github.com/dave/dst" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" ) @@ -131,10 +132,9 @@ func (enum *EnumType) createValueDeclaration(name TypeName, value EnumValue) dst } // AsType implements Type for EnumType -func (enum *EnumType) AsType(codeGenerationContext *CodeGenerationContext) dst.Expr { - // this should "never" happen as we name all enums; warn about it if it does - klog.Warning("Emitting unnamed enum, something’s awry") - return enum.baseType.AsType(codeGenerationContext) +func (enum *EnumType) AsType(_ *CodeGenerationContext) dst.Expr { + // this should "never" happen as we name all enums; panic if it does + panic(errors.New("Emitting unnamed enum, something’s awry")) } // AsZero renders an expression for the "zero" value of the type, diff --git a/v2/tools/generator/internal/astmodel/errored_type.go b/v2/tools/generator/internal/astmodel/errored_type.go index aca458ba7f3..fe6899f7f07 100644 --- a/v2/tools/generator/internal/astmodel/errored_type.go +++ b/v2/tools/generator/internal/astmodel/errored_type.go @@ -12,7 +12,6 @@ import ( "github.com/dave/dst" "github.com/pkg/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" ) type ErroredType struct { @@ -130,21 +129,25 @@ func (e *ErroredType) RequiredPackageReferences() *PackageReferenceSet { } func (e *ErroredType) handleWarningsAndErrors() { - for _, warning := range e.warnings { - klog.Warning(warning) + var errs []error + + // Treating warnings as errors isn't quite right, but good enough for now + if len(e.warnings) > 0 { + for _, wrn := range e.warnings { + errs = append(errs, errors.New(wrn)) + } } if len(e.errors) > 0 { - var errs []error for _, err := range e.errors { errs = append(errs, errors.New(err)) } + } - if len(errs) == 1 { - panic(errs[0]) - } else { - panic(kerrors.NewAggregate(errs)) - } + if len(errs) == 1 { + panic(errs[0]) + } else { + panic(kerrors.NewAggregate(errs)) } } From 86f0a1be84b3b3e14dda0c78353d372092daee3c Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 15:23:15 +1200 Subject: [PATCH 09/21] Generate logs during initial loading --- .../internal/codegen/pipeline/load_types.go | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/v2/tools/generator/internal/codegen/pipeline/load_types.go b/v2/tools/generator/internal/codegen/pipeline/load_types.go index fa7d68dab2f..6eb31d209a7 100644 --- a/v2/tools/generator/internal/codegen/pipeline/load_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/load_types.go @@ -14,6 +14,7 @@ import ( "sort" "strings" "sync" + "time" "github.com/go-logr/logr" "github.com/go-openapi/spec" @@ -318,7 +319,14 @@ func loadSwaggerData( loader := jsonast.NewCachingFileLoader(schemas) typesByGroup := make(map[astmodel.LocalPackageReference][]typesFromFile) + countLoaded := 0 for schemaPath, schema := range schemas { + logInfoSparse( + log, + "Loading Swagger files", + "loaded", countLoaded, + "total", len(schemas)) + countLoaded++ extractor := jsonast.NewSwaggerTypeExtractor( config, @@ -337,9 +345,31 @@ func loadSwaggerData( typesByGroup[*schema.Package] = append(typesByGroup[*schema.Package], typesFromFile{types, schemaPath}) } + log.Info( + "Loaded Swagger files", + "loaded", countLoaded, + "total", len(schemas)) + return mergeSwaggerTypesByGroup(idFactory, typesByGroup) } +var ( + loadingLock sync.Mutex + lastLogTime *time.Time +) + +func logInfoSparse(log logr.Logger, message string, keysAndValues ...interface{}) { + loadingLock.Lock() + defer loadingLock.Unlock() + + shouldDisplay := lastLogTime == nil || time.Since(*lastLogTime) > 800*time.Millisecond + if shouldDisplay { + log.Info(message, keysAndValues...) + now := time.Now() + lastLogTime = &now + } +} + func mergeSwaggerTypesByGroup(idFactory astmodel.IdentifierFactory, m map[astmodel.LocalPackageReference][]typesFromFile) (jsonast.SwaggerTypes, error) { result := jsonast.SwaggerTypes{ ResourceDefinitions: make(jsonast.ResourceDefinitionSet), @@ -643,7 +673,11 @@ func loadAllSchemas( var mutex sync.Mutex schemas := make(map[string]jsonast.PackageAndSwagger) + log.Info("Loading schemas", "rootPath", rootPath) var eg errgroup.Group + eg.SetLimit(10) + + countFound := 0 err := filepath.Walk(rootPath, func(filePath string, fileInfo os.FileInfo, err error) error { if err != nil { return err @@ -673,6 +707,12 @@ func loadAllSchemas( version) // all files are loaded in parallel to speed this up + logInfoSparse( + log, + "Scanning for schemas", + "found", countFound) + countFound++ + eg.Go(func() error { var swagger spec.Swagger @@ -702,6 +742,9 @@ func loadAllSchemas( }) egErr := eg.Wait() // for files to finish loading + log.Info( + "Scanning for schemas", + "found", countFound) if err != nil { return nil, err From d3c8052bc3249363dd032abff581710e1011d2b3 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 15:23:57 +1200 Subject: [PATCH 10/21] Code Gardening --- .../codegen/pipeline/create_arm_types.go | 2 +- .../internal/jsonast/openapi_file_loader.go | 4 +++- .../internal/jsonast/swagger_type_extractor.go | 18 ++++++++++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go b/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go index 7c8530125b0..70d42630568 100644 --- a/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/create_arm_types.go @@ -184,7 +184,7 @@ func (c *armTypeCreator) createARMTypeDefinition(isSpecType bool, def astmodel.T addOneOfConversionFunctionIfNeeded := func(t *astmodel.ObjectType) (*astmodel.ObjectType, error) { if isOneOf { - c.log.Info( + c.log.V(1).Info( "Adding MarshalJSON and UnmarshalJSON to OneOf", "type", def.Name()) marshal := functions.NewOneOfJSONMarshalFunction(t, c.idFactory) diff --git a/v2/tools/generator/internal/jsonast/openapi_file_loader.go b/v2/tools/generator/internal/jsonast/openapi_file_loader.go index eeb100062f3..20fd28c06a3 100644 --- a/v2/tools/generator/internal/jsonast/openapi_file_loader.go +++ b/v2/tools/generator/internal/jsonast/openapi_file_loader.go @@ -43,7 +43,9 @@ func NewCachingFileLoader(specs map[string]PackageAndSwagger) CachingFileLoader files[filepath.ToSlash(specPath)] = spec } - return CachingFileLoader{files} + return CachingFileLoader{ + files: files, + } } func (fileCache CachingFileLoader) knownFiles() []string { diff --git a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go index 263893fe4f8..521cf66f6d0 100644 --- a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go +++ b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go @@ -8,6 +8,8 @@ package jsonast import ( "context" "fmt" + "os" + "path/filepath" "regexp" "strings" @@ -162,9 +164,17 @@ func (extractor *SwaggerTypeExtractor) extractOneResourceType( armType, resourceName, err := extractor.resourceNameFromOperationPath(operationPath) if err != nil { // Logging using Info() because this is common and Error() can't be suppressed + // Convert swaggerPath to a relative path for readability + dir := extractor.swaggerPath + if cwd, err := os.Getwd(); err == nil { + if d, err := filepath.Rel(cwd, extractor.swaggerPath); err == nil { + dir = d + } + } + extractor.log.V(1).Info( "Error extracting resource name", - "swaggerPath", extractor.swaggerPath, + "swaggerPath", dir, "error", err) return nil } @@ -357,11 +367,11 @@ func (extractor *SwaggerTypeExtractor) findARMResourceSchema(op spec.PathItem, r if foundSpec == nil { if noBody { - extractor.log.V(2).Info( + extractor.log.V(1).Info( "no body parameter found for PUT operation", "operation", rawOperationPath) } else { - extractor.log.Info( + extractor.log.V(1).Info( "no schema found for PUT operation", "operation", rawOperationPath, "swagger", extractor.swaggerPath) @@ -457,7 +467,7 @@ func (extractor *SwaggerTypeExtractor) doesResponseRepresentARMResource(response return &schema, isMarkedAsARMResource(schema) } - extractor.log.Info( + extractor.log.V(1).Info( "no schema found for response", "operation", rawOperationPath, "swagger", extractor.swaggerPath) From 37151d743951eb941e93f40a3b52632b5ac8c7b5 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 15:33:18 +1200 Subject: [PATCH 11/21] Remove klog at entry points --- v2/tools/generator/gen_kustomize.go | 53 ++++++++++++++++------------- v2/tools/generator/gen_types.go | 3 +- v2/tools/generator/main.go | 2 -- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/v2/tools/generator/gen_kustomize.go b/v2/tools/generator/gen_kustomize.go index 82644c748de..af174445ace 100644 --- a/v2/tools/generator/gen_kustomize.go +++ b/v2/tools/generator/gen_kustomize.go @@ -6,15 +6,12 @@ package main import ( - "fmt" - "io/ioutil" "os" "path/filepath" "github.com/pkg/errors" "github.com/spf13/cobra" kerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/kustomization" ) @@ -34,22 +31,28 @@ func NewGenKustomizeCommand() (*cobra.Command, error) { const bases = "bases" const patches = "patches" + log := CreateLogger() + // We have an expectation that the folder structure is: .../config/crd/generated/bases and .../config/crd/generated/patches basesPath := filepath.Join(crdPath, bases) patchesPath := filepath.Join(crdPath, patches) destination := filepath.Join(crdPath, "kustomization.yaml") - klog.V(3).Infof("Scanning %q for resources", basesPath) + log.Info( + "Scanning for resources", + "basePath", basesPath) - files, err := ioutil.ReadDir(basesPath) + files, err := os.ReadDir(basesPath) if err != nil { - return logAndExtractStack(fmt.Sprintf("Unable to scan folder %q", basesPath), err) + log.Error(err, "Unable to scan folder", "folder", basesPath) + return err } err = os.MkdirAll(patchesPath, os.ModePerm) if err != nil { - return logAndExtractStack(fmt.Sprintf("Unable to create output folder %s", patchesPath), err) + log.Error(err, "Unable to create output folder", "folder", patchesPath) + return err } var errs []error @@ -60,7 +63,9 @@ func NewGenKustomizeCommand() (*cobra.Command, error) { continue } - klog.V(3).Infof("Found resource file %s", f.Name()) + log.V(1).Info( + "Found resource file", + "file", f.Name()) patchFile := "webhook-conversion-" + f.Name() var def *kustomization.ResourceDefinition @@ -70,7 +75,9 @@ func NewGenKustomizeCommand() (*cobra.Command, error) { continue } - klog.V(4).Infof("Resource is %q", def.Name()) + log.V(1).Info( + "Loaded Resource", + "name", def.Name()) patch := kustomization.NewConversionPatchFile(def.Name()) err = patch.Save(filepath.Join(patchesPath, patchFile)) @@ -84,17 +91,28 @@ func NewGenKustomizeCommand() (*cobra.Command, error) { } if len(errs) > 0 { - return logAndExtractStack("Error creating conversion patches", kerrors.NewAggregate(errs)) + err = kerrors.NewAggregate(errs) + log.Error( + err, + "Error creating conversion patches") + return err } if len(result.Resources) == 0 { err = errors.Errorf("no files found in %q", basesPath) - return logAndExtractStack("No CRD files found", err) + log.Error( + err, + "No CRD files found") + return err } err = result.Save(destination) if err != nil { - return logAndExtractStack("Error generating "+destination, err) + log.Error( + err, + "Error generating", + "destination", destination) + return err } return nil @@ -103,14 +121,3 @@ func NewGenKustomizeCommand() (*cobra.Command, error) { return cmd, nil } - -func logAndExtractStack(str string, err error) error { - klog.Errorf("%s:\n%s\n", str, err) - if tr, ok := findDeepestTrace(err); ok { - for _, fr := range tr { - klog.Errorf("%n (%s:%d)", fr, fr, fr) - } - } - - return err -} diff --git a/v2/tools/generator/gen_types.go b/v2/tools/generator/gen_types.go index 7e13874abfc..e7a175a1658 100644 --- a/v2/tools/generator/gen_types.go +++ b/v2/tools/generator/gen_types.go @@ -62,7 +62,8 @@ func NewGenTypesCommand() (*cobra.Command, error) { err = cg.Generate(ctx, log) if err != nil { - return logAndExtractStack("Error during code generation", err) + log.Error(err, "Error generating code generation") + return err } return nil diff --git a/v2/tools/generator/main.go b/v2/tools/generator/main.go index f00ceda2c8d..7314dbb6f2f 100644 --- a/v2/tools/generator/main.go +++ b/v2/tools/generator/main.go @@ -10,12 +10,10 @@ import ( "os" flag "github.com/spf13/pflag" - "k8s.io/klog/v2" ) func main() { flagSet := goflag.NewFlagSet(os.Args[0], goflag.ExitOnError) - klog.InitFlags(flagSet) flagSet.Parse(os.Args[1:]) //nolint:errcheck // error will never be returned due to ExitOnError flag.CommandLine.AddGoFlagSet(flagSet) Execute() From fd59e61c65272ac63fd0f1b91bab314383da2dad Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 20 Apr 2023 14:47:22 +1200 Subject: [PATCH 12/21] Add new packages and update dependencies --- v2/tools/generator/go.mod | 47 +++++++------ v2/tools/generator/go.sum | 141 ++++++++++++++++++-------------------- 2 files changed, 91 insertions(+), 97 deletions(-) diff --git a/v2/tools/generator/go.mod b/v2/tools/generator/go.mod index 2cd3cd83f92..5f7d858c694 100644 --- a/v2/tools/generator/go.mod +++ b/v2/tools/generator/go.mod @@ -9,47 +9,50 @@ replace github.com/Azure/azure-service-operator/v2 => ../../ replace github.com/xeipuuv/gojsonschema => github.com/devigned/gojsonschema v1.2.1-0.20191231010529-c593123f1e5d require ( - github.com/Azure/azure-service-operator/v2 v2.0.0-00010101000000-000000000000 + github.com/Azure/azure-service-operator/v2 v2.0.0 github.com/bmatcuk/doublestar v1.3.4 - github.com/dave/dst v0.26.2 + github.com/dave/dst v0.27.2 github.com/devigned/tab v0.1.1 - github.com/go-openapi/jsonpointer v0.19.5 - github.com/go-openapi/spec v0.20.4 + github.com/go-logr/logr v1.2.4 + github.com/go-logr/zerologr v1.2.3 + github.com/go-openapi/jsonpointer v0.19.6 + github.com/go-openapi/spec v0.20.9 github.com/gobuffalo/flect v1.0.2 github.com/google/go-cmp v0.5.9 github.com/hbollon/go-edlib v1.6.0 - github.com/kr/pretty v0.3.0 + github.com/kr/pretty v0.3.1 github.com/kylelemons/godebug v1.1.0 github.com/leanovate/gopter v0.2.9 - github.com/onsi/gomega v1.20.1 + github.com/onsi/gomega v1.27.6 github.com/pkg/errors v0.9.1 + github.com/rs/zerolog v1.29.1 github.com/sebdah/goldie/v2 v2.5.3 - github.com/spf13/cobra v1.6.1 + github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/xeipuuv/gojsonschema v1.2.0 - golang.org/x/exp v0.0.0-20220414153411-bcd21879b8fd - golang.org/x/mod v0.8.0 - golang.org/x/net v0.9.0 - golang.org/x/sync v0.1.0 + golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea + golang.org/x/mod v0.10.0 + golang.org/x/net v0.10.0 + golang.org/x/sync v0.2.0 + golang.org/x/text v0.9.0 gopkg.in/yaml.v3 v3.0.1 - k8s.io/apimachinery v0.25.2 - k8s.io/klog/v2 v2.80.1 + k8s.io/apimachinery v0.27.1 ) require ( - github.com/go-logr/logr v1.2.3 // indirect - github.com/go-openapi/jsonreference v0.20.0 // indirect + github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect - github.com/inconshreveable/mousetrap v1.0.1 // indirect + github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/kr/text v0.2.0 // indirect github.com/mailru/easyjson v0.7.7 // indirect + github.com/mattn/go-colorable v0.1.13 // indirect + github.com/mattn/go-isatty v0.0.18 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/rogpeppe/go-internal v1.6.1 // indirect - github.com/sergi/go-diff v1.0.0 // indirect - github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect + github.com/rogpeppe/go-internal v1.10.0 // indirect + github.com/sergi/go-diff v1.3.1 // indirect + github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect - golang.org/x/sys v0.7.0 // indirect - golang.org/x/text v0.9.0 // indirect - golang.org/x/tools v0.6.0 // indirect + golang.org/x/sys v0.8.0 // indirect + golang.org/x/tools v0.9.1 // indirect ) diff --git a/v2/tools/generator/go.sum b/v2/tools/generator/go.sum index 4c3438917c6..a18504c810e 100644 --- a/v2/tools/generator/go.sum +++ b/v2/tools/generator/go.sum @@ -1,15 +1,11 @@ -github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= -github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/bmatcuk/doublestar v1.3.4 h1:gPypJ5xD31uhX6Tf54sDPUOBXTqKH4c9aPY66CyQrS0= github.com/bmatcuk/doublestar v1.3.4/go.mod h1:wiQtGV+rzVYxB7WIlirSN++5HPtPlXEo9MEoZQC/PmE= +github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/dave/dst v0.26.2 h1:lnxLAKI3tx7MgLNVDirFCsDTlTG9nKTk7GcptKcWSwY= -github.com/dave/dst v0.26.2/go.mod h1:UMDJuIRPfyUCC78eFuB+SV/WI8oDeyFDvM/JR6NI3IU= -github.com/dave/gopackages v0.0.0-20170318123100-46e7023ec56e/go.mod h1:i00+b/gKdIDIxuLDFob7ustLAVqhsZRk2qVZrArELGQ= -github.com/dave/jennifer v1.2.0/go.mod h1:fIb+770HOpJ2fmN9EPPKOqm1vMGhB+TwXKMZhrIygKg= -github.com/dave/kerr v0.0.0-20170318121727-bc25dd6abe8e/go.mod h1:qZqlPyPvfsDJt+3wHJ1EvSXDuVjFTK0j2p/ca+gtsb8= -github.com/dave/rebecca v0.9.1/go.mod h1:N6XYdMD/OKw3lkF3ywh8Z6wPGuwNFDNtWYEMFWEmXBA= +github.com/dave/dst v0.27.2 h1:4Y5VFTkhGLC1oddtNwuxxe36pnyLxMFXT51FOzH8Ekc= +github.com/dave/dst v0.27.2/go.mod h1:jHh6EOibnHgcUW3WjKHisiooEkYwqpHLBSX1iOBhEyc= +github.com/dave/jennifer v1.5.0 h1:HmgPN93bVDpkQyYbqhCHj5QlgvUkvEOzMyEvKLgCRrg= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -17,36 +13,40 @@ github.com/devigned/gojsonschema v1.2.1-0.20191231010529-c593123f1e5d h1:0q2Xu1e github.com/devigned/gojsonschema v1.2.1-0.20191231010529-c593123f1e5d/go.mod h1:sSkPisfU5IfuoRgwlKZxezXDlqL0o2ocuSNwPqU5ecE= github.com/devigned/tab v0.1.1 h1:3mD6Kb1mUOYeLpJvTVSDwSg5ZsfSxfvxGRTxRsJsITA= github.com/devigned/tab v0.1.1/go.mod h1:XG9mPq0dFghrYvoBF3xdRrJzSTX1b7IQrvaL9mzjeJY= -github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= -github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ= +github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/zerologr v1.2.3 h1:up5N9vcH9Xck3jJkXzgyOxozT14R47IyDODz8LM1KSs= +github.com/go-logr/zerologr v1.2.3/go.mod h1:BxwGo7y5zgSHYR1BjbnHPyF/5ZjVKfKxAZANVu6E8Ho= github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= -github.com/go-openapi/jsonpointer v0.19.5 h1:gZr+CIYByUqjcgeLXnQu2gHYQC9o73G2XUeOFYEICuY= github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= -github.com/go-openapi/jsonreference v0.19.6/go.mod h1:diGHMEHg2IqXZGKxqyvWdfWU/aim5Dprw5bqpKkTvns= -github.com/go-openapi/jsonreference v0.20.0 h1:MYlu0sBgChmCfJxxUKZ8g1cPWFOB37YSZqewK7OKeyA= +github.com/go-openapi/jsonpointer v0.19.6 h1:eCs3fxoIi3Wh6vtgmLTOjdhSpiqphQ+DaPn38N2ZdrE= +github.com/go-openapi/jsonpointer v0.19.6/go.mod h1:osyAmYz/mB/C3I+WsTTSgw1ONzaLJoLCyoi6/zppojs= github.com/go-openapi/jsonreference v0.20.0/go.mod h1:Ag74Ico3lPc+zR+qjn4XBUmXymS4zJbYVCZmcgkasdo= -github.com/go-openapi/spec v0.20.4 h1:O8hJrt0UMnhHcluhIdUgCLRWyM2x7QkBXRvOs7m+O1M= -github.com/go-openapi/spec v0.20.4/go.mod h1:faYFR1CvsJZ0mNsmsphTMSoRrNV3TEDoAM7FOEWeq8I= +github.com/go-openapi/jsonreference v0.20.2 h1:3sVjiK66+uXK/6oQ8xgcRKcFgQ5KXa2KvnJRumpMGbE= +github.com/go-openapi/jsonreference v0.20.2/go.mod h1:Bl1zwGIM8/wsvqjsOQLJ/SH+En5Ap4rVB5KVcIDZG2k= +github.com/go-openapi/spec v0.20.9 h1:xnlYNQAwKd2VQRRfwTEI0DcK+2cbuvI/0c7jx3gA8/8= +github.com/go-openapi/spec v0.20.9/go.mod h1:2OpW+JddWPrpXSCIX8eOx7lZ5iyuWj3RYR6VaaBKcWA= github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= github.com/go-openapi/swag v0.19.15/go.mod h1:QYRuS/SOXUCsnplDa677K7+DxSOj6IPNl/eQntq43wQ= github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/g= github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= +github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= github.com/gobuffalo/flect v1.0.2 h1:eqjPGSo2WmjgY2XlpGwo2NXgL3RucAKo4k4qQMNA5sA= github.com/gobuffalo/flect v1.0.2/go.mod h1:A5msMlrHtLqh9umBSnvabjsMrCcCpAyzglnDvkbYKHs= +github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/pprof v0.0.0-20181127221834-b4f47329b966/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= +github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 h1:K6RDEckDVWvDI9JAJYCmNdQXq6neHJOYx3V6jnqNEec= github.com/hbollon/go-edlib v1.6.0 h1:ga7AwwVIvP8mHm9GsPueC0d71cfRU/52hmPJ7Tprv4E= github.com/hbollon/go-edlib v1.6.0/go.mod h1:wnt6o6EIVEzUfgbUZY7BerzQ2uvzp354qmS2xaLkrhM= -github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= -github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc= -github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= +github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= -github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -60,85 +60,78 @@ github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= +github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.18 h1:DOKFKCQ7FNG2L1rbrmstDN4QVRdS89Nkh85u68Uwp98= +github.com/mattn/go-isatty v0.0.18/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= -github.com/onsi/ginkgo/v2 v2.1.6 h1:Fx2POJZfKRQcM1pH49qSZiYeu319wji004qX+GDovrU= -github.com/onsi/gomega v1.20.1 h1:PA/3qinGoukvymdIDV8pii6tiZgC8kbmJO6Z5+b002Q= -github.com/onsi/gomega v1.20.1/go.mod h1:DtrZpjmvpn2mPm4YWQa0/ALMDj9v4YxLgojwPeREyVo= +github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= +github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= +github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= -github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= +github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= +github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= +github.com/rs/xid v1.4.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= +github.com/rs/zerolog v1.29.1 h1:cO+d60CHkknCbvzEWxP0S9K6KqyTjrCNUy1LdQLCGPc= +github.com/rs/zerolog v1.29.1/go.mod h1:Le6ESbR7hc+DP6Lt1THiV8CQSdkkNrd3R0XbEgp3ZBU= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sebdah/goldie/v2 v2.5.3 h1:9ES/mNN+HNUbNWpVAlrzuZ7jE+Nrczbj8uFRjM7624Y= github.com/sebdah/goldie/v2 v2.5.3/go.mod h1:oZ9fp0+se1eapSRjfYbsV/0Hqhbuu3bJVvKI/NNtssI= -github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ= github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= -github.com/spf13/cobra v1.6.1 h1:o94oiPyS4KD1mPy2fmcYYHHfCxLqYjJOhGsCHFZtEzA= -github.com/spf13/cobra v1.6.1/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY= +github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8= +github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I= +github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= +github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= +github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo= +github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= -github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -golang.org/x/arch v0.0.0-20180920145803-b19384d3c130/go.mod h1:cYlCBUl1MsqxdiKgmc4uh7TxZfWSFLOGSRR090WDxt8= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/exp v0.0.0-20220414153411-bcd21879b8fd h1:zVFyTKZN/Q7mNRWSs1GOYnHM9NiFSJ54YVRsD0rNWT4= -golang.org/x/exp v0.0.0-20220414153411-bcd21879b8fd/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE= -golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= -golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20210421230115-4e50805a0758/go.mod h1:72T/g9IO56b78aLF+1Kcs5dz7/ng1VjMUvfKvpfy+jM= -golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM= -golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= -golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20180903190138-2b024373dcd9/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210420072515-93ed5bcd2bfe/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= -golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea h1:vLCWI/yYrdEHyN2JzIzPO3aaQJHQdp89IZBA/+azVC4= +golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= +golang.org/x/mod v0.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk= +golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M= +golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= +golang.org/x/sync v0.2.0 h1:PUR+T4wwASmuSTYdKjYHI5TD22Wy5ogLU5qZCOLxBrI= +golang.org/x/sync v0.2.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= +golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= -golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo= +golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= -gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= -gopkg.in/src-d/go-billy.v4 v4.3.0/go.mod h1:tm33zBoOwxjYHZIE+OV8bxTWFMJLrconzFMd38aARFk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= @@ -146,7 +139,5 @@ gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -k8s.io/apimachinery v0.25.2 h1:WbxfAjCx+AeN8Ilp9joWnyJ6xu9OMeS/fsfjK/5zaQs= -k8s.io/apimachinery v0.25.2/go.mod h1:hqqA1X0bsgsxI6dXsJ4HnNTBOmJNxyPp8dw3u2fSHwA= -k8s.io/klog/v2 v2.80.1 h1:atnLQ121W371wYYFawwYx1aEY2eUfs4l3J72wtgAwV4= -k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= +k8s.io/apimachinery v0.27.1 h1:EGuZiLI95UQQcClhanryclaQE6xjg1Bts6/L3cD7zyc= +k8s.io/apimachinery v0.27.1/go.mod h1:5ikh59fK3AJ287GUvpUsryoMFtH9zj/ARfWCo3AyXTM= From d6f6a23b0dd0ff74c14b739e50cf636dddb0f992 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 17:32:09 +1200 Subject: [PATCH 13/21] Update tests --- v2/tools/generator/internal/codegen/golden_files_test.go | 6 +++--- .../internal/codegen/pipeline/create_arm_types_test.go | 8 ++++---- .../generator/internal/codegen/pipeline_factory_test.go | 5 +++-- ...wARMCodeGeneratorFromConfigCreatesRightPipeline.golden | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/v2/tools/generator/internal/codegen/golden_files_test.go b/v2/tools/generator/internal/codegen/golden_files_test.go index cadf94cd018..ce004eac68b 100644 --- a/v2/tools/generator/internal/codegen/golden_files_test.go +++ b/v2/tools/generator/internal/codegen/golden_files_test.go @@ -122,7 +122,7 @@ func runGoldenTest(t *testing.T, path string, testConfig GoldenTestConfig) { t.Fatalf("failed to create code generator: %s", err) } - err = codegen.Generate(ctx) + err = codegen.Generate(ctx, logr.Discard()) if err != nil { t.Fatalf("codegen failed: %s", err) } @@ -146,7 +146,7 @@ func NewTestCodeGenerator( return nil, err } - codegen, err := NewTargetedCodeGeneratorFromConfig(cfg, idFactory, pipelineTarget) + codegen, err := NewTargetedCodeGeneratorFromConfig(cfg, idFactory, pipelineTarget, logr.Discard()) if err != nil { return nil, err } @@ -344,7 +344,7 @@ func addCrossResourceReferencesForTest(idFactory astmodel.IdentifierFactory) *pi return pipeline.ARMIDPropertyClassificationUnspecified } - crossReferenceVisitor := pipeline.MakeARMIDPropertyTypeVisitor(isCrossResourceReference) + crossReferenceVisitor := pipeline.MakeARMIDPropertyTypeVisitor(isCrossResourceReference, logr.Discard()) resourceReferenceVisitor := pipeline.MakeARMIDToResourceReferenceTypeVisitor(idFactory) for _, def := range state.Definitions() { diff --git a/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go b/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go index ab8229c3ea4..f9312a54346 100644 --- a/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go @@ -44,7 +44,7 @@ func TestCreateFlattenedARMType_CreatesExpectedConversions(t *testing.T) { idFactory := astmodel.NewIdentifierFactory() - createARMTypes := CreateARMTypes(idFactory) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) flatten := FlattenProperties() simplify := SimplifyDefinitions() @@ -100,7 +100,7 @@ func TestCreateFlattenedARMTypeWithResourceRef_CreatesExpectedConversions(t *tes configToARMIDs := ApplyCrossResourceReferencesFromConfig(configuration, logr.Discard()) crossResourceRefs := TransformCrossResourceReferences(configuration, idFactory) - createARMTypes := CreateARMTypes(idFactory) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) flatten := FlattenProperties() simplify := SimplifyDefinitions() @@ -167,7 +167,7 @@ func TestCreateFlattenedARMTypeWithConfigMap_CreatesExpectedConversions(t *testi configuration.ObjectModelConfiguration = omc addConfigMaps := AddConfigMaps(configuration) - createARMTypes := CreateARMTypes(idFactory) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) flatten := FlattenProperties() simplify := SimplifyDefinitions() @@ -243,7 +243,7 @@ func TestCreateARMTypeWithConfigMap_CreatesExpectedConversions(t *testing.T) { configuration.ObjectModelConfiguration = omc addConfigMaps := AddConfigMaps(configuration) - createARMTypes := CreateARMTypes(idFactory) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() diff --git a/v2/tools/generator/internal/codegen/pipeline_factory_test.go b/v2/tools/generator/internal/codegen/pipeline_factory_test.go index 8d2dc3cbe60..d89c66e4498 100644 --- a/v2/tools/generator/internal/codegen/pipeline_factory_test.go +++ b/v2/tools/generator/internal/codegen/pipeline_factory_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/go-logr/logr" "github.com/sebdah/goldie/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -28,7 +29,7 @@ func TestGolden_NewARMCodeGeneratorFromConfigCreatesRightPipeline(t *testing.T) idFactory := astmodel.NewIdentifierFactory() configuration := config.NewConfiguration() - codegen, err := NewTargetedCodeGeneratorFromConfig(configuration, idFactory, pipeline.ARMTarget) + codegen, err := NewTargetedCodeGeneratorFromConfig(configuration, idFactory, pipeline.ARMTarget, logr.Discard()) g.Expect(err).To(Succeed()) result := writePipeline("Expected Pipeline Stages for ARM Code Generation", codegen) @@ -46,7 +47,7 @@ func TestGolden_NewCrossplaneCodeGeneratorFromConfigCreatesRightPipeline(t *test idFactory := astmodel.NewIdentifierFactory() configuration := config.NewConfiguration() - codegen, err := NewTargetedCodeGeneratorFromConfig(configuration, idFactory, pipeline.CrossplaneTarget) + codegen, err := NewTargetedCodeGeneratorFromConfig(configuration, idFactory, pipeline.CrossplaneTarget, logr.Discard()) g.Expect(err).To(Succeed()) result := writePipeline("Expected Pipeline Stages for ARM Code Generation", codegen) diff --git a/v2/tools/generator/internal/codegen/testdata/TestGolden_NewARMCodeGeneratorFromConfigCreatesRightPipeline.golden b/v2/tools/generator/internal/codegen/testdata/TestGolden_NewARMCodeGeneratorFromConfigCreatesRightPipeline.golden index b983b33d582..c481693f97a 100644 --- a/v2/tools/generator/internal/codegen/testdata/TestGolden_NewARMCodeGeneratorFromConfigCreatesRightPipeline.golden +++ b/v2/tools/generator/internal/codegen/testdata/TestGolden_NewARMCodeGeneratorFromConfigCreatesRightPipeline.golden @@ -18,7 +18,7 @@ removeAliases Remove type aliases collapseCrossGroupReferences Find and remove cross group references stripUnreferenced Strip unreferenced types assertTypesStructureValid Verify that all local TypeNames refer to a type -removeEmbeddedResources azure Remove properties that point to embedded resources. Only removes structural aspects of embedded resources, Id/ARMId references are retained. +removeEmbeddedResources azure Remove properties that point to embedded resources. filterTypes Apply export filters to reduce the number of generated types add-api-version-enums Add enums for API Versions in each package removeAliases Remove type aliases From 88b685084bd77feebdeda8cfeda3d18a261f44ba Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 17:43:11 +1200 Subject: [PATCH 14/21] Address lint issues --- .../generator/internal/jsonast/swagger_type_extractor.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go index 521cf66f6d0..143fcabfab1 100644 --- a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go +++ b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go @@ -159,15 +159,15 @@ func (extractor *SwaggerTypeExtractor) extractOneResourceType( specSchema *Schema, statusSchema *Schema, nameParameterType astmodel.Type, - operationPath string) error { - + operationPath string, +) error { armType, resourceName, err := extractor.resourceNameFromOperationPath(operationPath) if err != nil { // Logging using Info() because this is common and Error() can't be suppressed // Convert swaggerPath to a relative path for readability dir := extractor.swaggerPath - if cwd, err := os.Getwd(); err == nil { - if d, err := filepath.Rel(cwd, extractor.swaggerPath); err == nil { + if cwd, osErr := os.Getwd(); osErr == nil { + if d, pathErr := filepath.Rel(cwd, extractor.swaggerPath); pathErr == nil { dir = d } } From f461998bec7d1895d06364c277de73ebe6f0177f Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Fri, 12 May 2023 18:47:32 +1200 Subject: [PATCH 15/21] Fix specifier --- v2/tools/generator/internal/codegen/code_generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index deaedf38c07..001c15dd850 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -345,7 +345,7 @@ func (generator *CodeGenerator) executeStage( if e, ok := r.(error); ok { err = e } else { - err = errors.Errorf("panic: %v", r) + err = errors.Errorf("panic: %s", r) } } }() From cabd956c4ecb0ade0dca6e38f41796b2a022b6d3 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Tue, 23 May 2023 04:06:00 +0000 Subject: [PATCH 16/21] Address PR feedback --- .../internal/astmodel/errored_type.go | 12 ++-- .../internal/codegen/code_generator.go | 26 ++------ .../codegen/embeddedresources/remover.go | 2 +- .../codegen/embeddedresources/renamer.go | 24 +++++++- .../codegen/embeddedresources/renamer_test.go | 9 +-- .../pipeline/apply_property_rewrites.go | 2 +- .../convert_allof_and_oneof_to_objects.go | 2 +- .../codegen/pipeline/flatten_properties.go | 61 +++++++++++-------- .../pipeline/flatten_properties_test.go | 5 +- .../simple_recursive_type_fixer.go | 10 ++- .../internal/config/type_transformer.go | 4 +- v2/tools/generator/root.go | 1 - 12 files changed, 93 insertions(+), 65 deletions(-) diff --git a/v2/tools/generator/internal/astmodel/errored_type.go b/v2/tools/generator/internal/astmodel/errored_type.go index fe6899f7f07..fe853fb9d46 100644 --- a/v2/tools/generator/internal/astmodel/errored_type.go +++ b/v2/tools/generator/internal/astmodel/errored_type.go @@ -131,6 +131,12 @@ func (e *ErroredType) RequiredPackageReferences() *PackageReferenceSet { func (e *ErroredType) handleWarningsAndErrors() { var errs []error + if len(e.errors) > 0 { + for _, err := range e.errors { + errs = append(errs, errors.New(err)) + } + } + // Treating warnings as errors isn't quite right, but good enough for now if len(e.warnings) > 0 { for _, wrn := range e.warnings { @@ -138,12 +144,6 @@ func (e *ErroredType) handleWarningsAndErrors() { } } - if len(e.errors) > 0 { - for _, err := range e.errors { - errs = append(errs, errors.New(err)) - } - } - if len(errs) == 1 { panic(errs[0]) } else { diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index 001c15dd850..a9a41828181 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -194,7 +194,7 @@ func createAllPipelineStages( pipeline.ApplyKubernetesResourceInterface(idFactory, log).UsedFor(pipeline.ARMTarget), // Effects the "flatten" property of Properties: - pipeline.FlattenProperties(), + pipeline.FlattenProperties(log), // Remove types which may not be needed after flattening pipeline.StripUnreferencedTypeDefinitions(), @@ -288,7 +288,6 @@ func (generator *CodeGenerator) Generate( newState, err := generator.executeStage(ctx, stageNumber, stage, state) if err != nil { - log.Error(err, "failed to execute stage", "stage", stage.Description()) return errors.Wrapf(err, "failed to execute stage %d: %s", stageNumber, stage.Description()) } @@ -297,29 +296,16 @@ func (generator *CodeGenerator) Generate( defsAdded := newState.Definitions().Except(state.Definitions()) defsRemoved := state.Definitions().Except(newState.Definitions()) - if len(defsAdded) > 0 && len(defsRemoved) > 0 { - log.Info( - stageDescription, - "elapsed", duration, - "added", len(defsAdded), - "removed", len(defsRemoved)) - } else if len(defsAdded) > 0 { - log.Info( - stageDescription, - "elapsed", duration, - "added", len(defsAdded)) - } else if len(defsRemoved) > 0 { - log.Info( - stageDescription, - "elapsed", duration, - "removed", len(defsRemoved)) - } + log.Info( + stageDescription, + "elapsed", duration, + "added", len(defsAdded), + "removed", len(defsRemoved)) state = newState } if err := state.CheckFinalState(); err != nil { - log.Error(err, "Final state check failed") return err } diff --git a/v2/tools/generator/internal/codegen/embeddedresources/remover.go b/v2/tools/generator/internal/codegen/embeddedresources/remover.go index a8cd784364d..17cfe749a68 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/remover.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/remover.go @@ -132,7 +132,7 @@ func (e EmbeddedResourceRemover) RemoveEmbeddedResources( } } - result, err := simplifyTypeNames(result, e.typeFlag, originalNames) + result, err := simplifyTypeNames(result, e.typeFlag, originalNames, log) if err != nil { return nil, err } diff --git a/v2/tools/generator/internal/codegen/embeddedresources/renamer.go b/v2/tools/generator/internal/codegen/embeddedresources/renamer.go index 28c6a409ffb..9b89320cf36 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/renamer.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/renamer.go @@ -8,6 +8,7 @@ package embeddedresources import ( "fmt" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -21,12 +22,14 @@ type renameAction func( original astmodel.TypeName, // Original name of the resource type associatedNames astmodel.TypeNameSet, // all the subresource names that copies have acquired originalNames map[astmodel.TypeName]embeddedResourceTypeName, // map from the new names back to the original + log logr.Logger, // Log to use for reporting ) (astmodel.TypeAssociation, error) func (r renamer) simplifyEmbeddedNameToOriginalName( original astmodel.TypeName, associatedNames astmodel.TypeNameSet, _ map[astmodel.TypeName]embeddedResourceTypeName, + log logr.Logger, ) (astmodel.TypeAssociation, error) { _, originalExists := r.definitions[original] if originalExists || len(associatedNames) != 1 { @@ -36,6 +39,11 @@ func (r renamer) simplifyEmbeddedNameToOriginalName( renames := make(astmodel.TypeAssociation) renames[associatedNames.Single()] = original + log.V(2).Info( + "Type not used, renaming for simplicity", + "originalName", associatedNames.Single(), + "newName", original) + return renames, nil } @@ -43,6 +51,7 @@ func (r renamer) simplifyEmbeddedNameRemoveContextAndCount( _ astmodel.TypeName, associatedNames astmodel.TypeNameSet, originalNames map[astmodel.TypeName]embeddedResourceTypeName, + log logr.Logger, ) (astmodel.TypeAssociation, error) { if len(associatedNames) != 1 { return nil, nil @@ -60,6 +69,11 @@ func (r renamer) simplifyEmbeddedNameRemoveContextAndCount( embeddedName.count = 0 renames[associated] = embeddedName.ToSimplifiedTypeName() + log.V(2).Info( + "Type used in single context, renaming for simplicity", + "originalName", associated, + "newName", renames[associated]) + return renames, nil } @@ -67,6 +81,7 @@ func (r renamer) simplifyEmbeddedNameRemoveContext( _ astmodel.TypeName, associatedNames astmodel.TypeNameSet, originalNames map[astmodel.TypeName]embeddedResourceTypeName, + log logr.Logger, ) (astmodel.TypeAssociation, error) { // Gather information about the associated definitions associatedCountPerContext := make(map[string]int, len(associatedNames)) @@ -91,6 +106,11 @@ func (r renamer) simplifyEmbeddedNameRemoveContext( } embeddedName.context = "" renames[associated] = embeddedName.ToSimplifiedTypeName() + + log.V(2).Info( + "Type used in single context, removing context from name", + "originalName", associated, + "newName", renames[associated]) } return renames, nil @@ -100,6 +120,7 @@ func (r renamer) simplifyEmbeddedName( _ astmodel.TypeName, associatedNames astmodel.TypeNameSet, originalNames map[astmodel.TypeName]embeddedResourceTypeName, + log logr.Logger, ) (astmodel.TypeAssociation, error) { // remove _0, which especially for the cases where there's only a single // kind of usage will make the type name much clearer @@ -156,6 +177,7 @@ func simplifyTypeNames( definitions astmodel.TypeDefinitionSet, flag astmodel.TypeFlag, originalNames map[astmodel.TypeName]embeddedResourceTypeName, + log logr.Logger, ) (astmodel.TypeDefinitionSet, error) { // Find all of the type names that have the flag we're interested in updatedNames := make(map[astmodel.TypeName]astmodel.TypeNameSet) @@ -185,7 +207,7 @@ func simplifyTypeNames( renames := make(astmodel.TypeAssociation) for original, associatedNames := range updatedNames { for _, action := range renameActions { - result, err := action(original, associatedNames, originalNames) + result, err := action(original, associatedNames, originalNames, log) if err != nil { return nil, err } diff --git a/v2/tools/generator/internal/codegen/embeddedresources/renamer_test.go b/v2/tools/generator/internal/codegen/embeddedresources/renamer_test.go index 85816030962..934cdd71864 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/renamer_test.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/renamer_test.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/test" + "github.com/go-logr/logr" . "github.com/onsi/gomega" ) @@ -163,7 +164,7 @@ func TestCleanupTypeNames_TypeWithNoOriginalName_UpdatedNameCollapsed(t *testing types, originalNames := typesWithSubresourceTypeNoOriginalNameUsage() - updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames) + updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames, logr.Discard()) g.Expect(err).ToNot(HaveOccurred()) g.Expect(len(updatedTypes)).To(Equal(2)) @@ -188,7 +189,7 @@ func TestCleanupTypeNames_TypeWithOriginalNameExists_UpdatedNamePartiallyCollaps expectedOriginalTypeName := newTestName("T1") types, originalNames := typesWithSubresourceTypeOriginalNameUsage() - updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames) + updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames, logr.Discard()) g.Expect(err).ToNot(HaveOccurred()) g.Expect(len(updatedTypes)).To(Equal(3)) @@ -217,7 +218,7 @@ func TestCleanupTypeNames_UpdatedNamesAreAllForSameResource_UpdatedNamesStripped expectedUpdatedTypeName2 := newTestName("T1_TestSuffix_1") types, originalNames := typesWithSubresourceTypeMultipleUsageContextsOneResource() - updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames) + updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames, logr.Discard()) g.Expect(err).ToNot(HaveOccurred()) g.Expect(len(updatedTypes)).To(Equal(3)) @@ -246,7 +247,7 @@ func TestCleanupTypeNames_UpdatedNamesAreEachForDifferentResource_UpdatedNamesSt expectedUpdatedTypeName2 := newTestName("T1_Resource2_TestSuffix") types, originalNames := typesWithSubresourceTypeMultipleResourcesOneUsageContextEach() - updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames) + updatedTypes, err := simplifyTypeNames(types, exampleTypeFlag, originalNames, logr.Discard()) g.Expect(err).ToNot(HaveOccurred()) g.Expect(len(updatedTypes)).To(Equal(3)) diff --git a/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go b/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go index 71223702e4b..742c2f33071 100644 --- a/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go +++ b/v2/tools/generator/internal/codegen/pipeline/apply_property_rewrites.go @@ -35,7 +35,7 @@ func ApplyPropertyRewrites( transformations := config.TransformTypeProperties(name, objectType) for _, transformation := range transformations { - transformation.Log(log) + transformation.LogTo(log) objectType = transformation.NewType } diff --git a/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go b/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go index 1e90f7aaa75..12557960217 100644 --- a/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go +++ b/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go @@ -601,7 +601,7 @@ func (s synthesizer) handleObjectObject( if !ok { // Property doesn't already exist, so just add it mergedProps[p.PropertyName()] = p - return nil + return nil // continue } newType, err := s.intersectTypes(existingProp.PropertyType(), p.PropertyType()) diff --git a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go index 2412afb7af8..70b1069b5ea 100644 --- a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go +++ b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go @@ -9,35 +9,37 @@ import ( "context" "strings" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) -func FlattenProperties() *Stage { - return NewLegacyStage("flattenProperties", "Apply flattening to properties marked for flattening", applyPropertyFlattening) -} - -func applyPropertyFlattening( - ctx context.Context, - defs astmodel.TypeDefinitionSet, -) (astmodel.TypeDefinitionSet, error) { - visitor := makeFlatteningVisitor(defs) - - result := make(astmodel.TypeDefinitionSet) - for name, def := range defs { - newDef, err := visitor.VisitDefinition(def, name) - if err != nil { - return nil, err - } +const FlattenPropertiesStageId = "flattenProperties" + +func FlattenProperties(log logr.Logger) *Stage { + return NewStage( + FlattenPropertiesStageId, + "Apply flattening to properties marked for flattening", + func(ctx context.Context, state *State) (*State, error) { + defs := state.Definitions() + visitor := makeFlatteningVisitor(defs, log) + + result := make(astmodel.TypeDefinitionSet) + for name, def := range defs { + newDef, err := visitor.VisitDefinition(def, name) + if err != nil { + return nil, err + } - result.Add(newDef) - } + result.Add(newDef) + } - return result, nil + return state.WithDefinitions(result), nil + }) } -func makeFlatteningVisitor(defs astmodel.TypeDefinitionSet) astmodel.TypeVisitor { +func makeFlatteningVisitor(defs astmodel.TypeDefinitionSet, log logr.Logger) astmodel.TypeVisitor { return astmodel.TypeVisitorBuilder{ VisitObjectType: func(this *astmodel.TypeVisitor, it *astmodel.ObjectType, ctx interface{}) (astmodel.Type, error) { name := ctx.(astmodel.TypeName) @@ -49,7 +51,7 @@ func makeFlatteningVisitor(defs astmodel.TypeDefinitionSet) astmodel.TypeVisitor it = newIt.(*astmodel.ObjectType) - newProps, err := collectAndFlattenProperties(name, it, defs) + newProps, err := collectAndFlattenProperties(name, it, defs, log) if err != nil { return nil, err } @@ -139,6 +141,7 @@ func collectAndFlattenProperties( container astmodel.TypeName, objectType *astmodel.ObjectType, defs astmodel.TypeDefinitionSet, + log logr.Logger, ) ([]*astmodel.PropertyDefinition, error) { var flattenedProps []*astmodel.PropertyDefinition @@ -151,6 +154,12 @@ func collectAndFlattenProperties( innerProps, err := flattenProperty(container, prop, defs) if err != nil { + log.V(2).Info( + "Skipping flatten", + "property", prop.PropertyName(), + "container", container, + "reason", err) + innerProps = []*astmodel.PropertyDefinition{ prop.SetFlatten(false), } @@ -166,8 +175,9 @@ func flattenProperty( container astmodel.TypeName, prop *astmodel.PropertyDefinition, defs astmodel.TypeDefinitionSet, + log logr.Logger, ) ([]*astmodel.PropertyDefinition, error) { - props, err := flattenPropType(container, prop.PropertyType(), defs) + props, err := flattenPropType(container, prop.PropertyType(), defs, log) if err != nil { return nil, errors.Wrapf(err, "flattening property %s", prop.PropertyName()) } @@ -184,11 +194,12 @@ func flattenPropType( container astmodel.TypeName, propType astmodel.Type, defs astmodel.TypeDefinitionSet, + log logr.Logger, ) ([]*astmodel.PropertyDefinition, error) { switch propType := propType.(type) { // "base case" case *astmodel.ObjectType: - return collectAndFlattenProperties(container, propType, defs) + return collectAndFlattenProperties(container, propType, defs, log) // typename must be resolved case astmodel.TypeName: @@ -197,7 +208,7 @@ func flattenPropType( return nil, err } - props, err := flattenPropType(container, resolved, defs) + props, err := flattenPropType(container, resolved, defs, log) if err != nil { return nil, err } @@ -206,7 +217,7 @@ func flattenPropType( // flattening something that is optional makes everything inside it optional case *astmodel.OptionalType: - innerProps, err := flattenPropType(container, propType.Element(), defs) + innerProps, err := flattenPropType(container, propType.Element(), defs, log) if err != nil { return nil, errors.Wrap(err, "wrapping optional type") } diff --git a/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go b/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go index 2d98d2a3bba..71ab335c72d 100644 --- a/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go @@ -9,6 +9,7 @@ import ( "context" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -31,7 +32,7 @@ func TestDuplicateNamesAreCaughtAndRenamed(t *testing.T) { defs := make(astmodel.TypeDefinitionSet) defs.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "ObjType"), objType)) - result, err := applyPropertyFlattening(context.Background(), defs) + result, err := applyPropertyFlattening(context.Background(), defs, logr.Discard()) // We don't fail but flattening does not occur, and flatten is set to false g.Expect(err).ToNot(HaveOccurred()) @@ -67,7 +68,7 @@ func TestFlatteningWorks(t *testing.T) { defs := make(astmodel.TypeDefinitionSet) defs.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "objType"), objType)) - result, err := applyPropertyFlattening(context.Background(), defs) + result, err := applyPropertyFlattening(context.Background(), defs, logr.Discard()) g.Expect(err).ToNot(HaveOccurred()) g.Expect(result).To(HaveLen(1)) diff --git a/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go b/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go index 9cdae9e0483..ac5ac37d107 100644 --- a/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go +++ b/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go @@ -7,6 +7,7 @@ package recursivetypefixer import ( "github.com/Azure/azure-service-operator/v2/internal/set" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -18,6 +19,7 @@ import ( type SimpleRecursiveTypeFixer struct { visitor *astmodel.TypeVisitor newDefinitions astmodel.TypeDefinitionSet + log logr.Logger } type simpleRecursiveTypeFixerContext struct { @@ -32,9 +34,10 @@ func (c simpleRecursiveTypeFixerContext) WithUnrolledName(name astmodel.TypeName return c } -func NewSimpleRecursiveTypeFixer() *SimpleRecursiveTypeFixer { +func NewSimpleRecursiveTypeFixer(log logr.Logger) *SimpleRecursiveTypeFixer { result := &SimpleRecursiveTypeFixer{ newDefinitions: make(astmodel.TypeDefinitionSet), + log: log, } visitor := astmodel.TypeVisitorBuilder{ @@ -101,6 +104,11 @@ func (s *SimpleRecursiveTypeFixer) unrollRecursiveReference(this *astmodel.TypeV return astmodel.IdentityVisitOfTypeName(this, it, ctx) } + s.log.V(2).Info( + "unrolling recursive reference", + "from", typedCtx.name, + "to", typedCtx.unrolledName) + return astmodel.IdentityVisitOfTypeName(this, typedCtx.unrolledName, typedCtx) } diff --git a/v2/tools/generator/internal/config/type_transformer.go b/v2/tools/generator/internal/config/type_transformer.go index 61eb24690de..3ccd869180d 100644 --- a/v2/tools/generator/internal/config/type_transformer.go +++ b/v2/tools/generator/internal/config/type_transformer.go @@ -334,8 +334,8 @@ func (r PropertyTransformResult) String() string { ) } -// Log creates a log message for the transformation -func (r PropertyTransformResult) Log(log logr.Logger) { +// LogTo creates a log message for the transformation +func (r PropertyTransformResult) LogTo(log logr.Logger) { if r.Removed { log.V(2).Info( "Removing property", diff --git a/v2/tools/generator/root.go b/v2/tools/generator/root.go index 90c3881f6a3..442166f2ecb 100644 --- a/v2/tools/generator/root.go +++ b/v2/tools/generator/root.go @@ -82,7 +82,6 @@ var ( ) // CreateLogger creates a logger for console output. -// Use this when your command wants to show only log messages func CreateLogger() logr.Logger { // Configure console writer for ZeroLog From 902fd2bb74a9e0676b657735fb49816c0a5531f6 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Tue, 23 May 2023 04:25:38 +0000 Subject: [PATCH 17/21] Fixup tests --- .../generator/internal/codegen/code_generator.go | 2 +- .../codegen/pipeline/create_arm_types_test.go | 6 +++--- .../codegen/pipeline/flatten_properties.go | 2 +- .../codegen/pipeline/flatten_properties_test.go | 16 +++++++++++----- .../codegen/pipeline/unroll_recursive_types.go | 5 +++-- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index a9a41828181..1f59e7f9ede 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -134,7 +134,7 @@ func createAllPipelineStages( pipeline.ApplyIsResourceOverrides(configuration), pipeline.FixIDFields(), - pipeline.UnrollRecursiveTypes(), + pipeline.UnrollRecursiveTypes(log), pipeline.RemoveStatusValidations(), // Figure out resource owners: diff --git a/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go b/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go index f9312a54346..f5dd0094330 100644 --- a/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/create_arm_types_test.go @@ -46,7 +46,7 @@ func TestCreateFlattenedARMType_CreatesExpectedConversions(t *testing.T) { createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) - flatten := FlattenProperties() + flatten := FlattenProperties(logr.Discard()) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() @@ -102,7 +102,7 @@ func TestCreateFlattenedARMTypeWithResourceRef_CreatesExpectedConversions(t *tes crossResourceRefs := TransformCrossResourceReferences(configuration, idFactory) createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) - flatten := FlattenProperties() + flatten := FlattenProperties(logr.Discard()) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() @@ -169,7 +169,7 @@ func TestCreateFlattenedARMTypeWithConfigMap_CreatesExpectedConversions(t *testi addConfigMaps := AddConfigMaps(configuration) createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) - flatten := FlattenProperties() + flatten := FlattenProperties(logr.Discard()) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() diff --git a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go index 70b1069b5ea..e21ae89920a 100644 --- a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go +++ b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go @@ -152,7 +152,7 @@ func collectAndFlattenProperties( return // continue } - innerProps, err := flattenProperty(container, prop, defs) + innerProps, err := flattenProperty(container, prop, defs, logr.Discard()) if err != nil { log.V(2).Info( "Skipping flatten", diff --git a/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go b/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go index 71ab335c72d..075768555b0 100644 --- a/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/flatten_properties_test.go @@ -32,7 +32,10 @@ func TestDuplicateNamesAreCaughtAndRenamed(t *testing.T) { defs := make(astmodel.TypeDefinitionSet) defs.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "ObjType"), objType)) - result, err := applyPropertyFlattening(context.Background(), defs, logr.Discard()) + state := NewState(defs) + stage := FlattenProperties(logr.Discard()) + + result, err := stage.Run(context.Background(), state) // We don't fail but flattening does not occur, and flatten is set to false g.Expect(err).ToNot(HaveOccurred()) @@ -47,7 +50,7 @@ func TestDuplicateNamesAreCaughtAndRenamed(t *testing.T) { expectedDefs := make(astmodel.TypeDefinitionSet) expectedDefs.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "ObjType"), newObjType)) - g.Expect(result).To(Equal(expectedDefs)) + g.Expect(result.Definitions()).To(Equal(expectedDefs)) } func TestFlatteningWorks(t *testing.T) { @@ -68,12 +71,15 @@ func TestFlatteningWorks(t *testing.T) { defs := make(astmodel.TypeDefinitionSet) defs.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "objType"), objType)) - result, err := applyPropertyFlattening(context.Background(), defs, logr.Discard()) + state := NewState(defs) + stage := FlattenProperties(logr.Discard()) + + result, err := stage.Run(context.Background(), state) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result).To(HaveLen(1)) + g.Expect(result.Definitions()).To(HaveLen(1)) var it astmodel.Type - for _, single := range result { + for _, single := range result.Definitions() { it = single.Type() break } diff --git a/v2/tools/generator/internal/codegen/pipeline/unroll_recursive_types.go b/v2/tools/generator/internal/codegen/pipeline/unroll_recursive_types.go index e0b7dc69425..ebcf65f47d7 100644 --- a/v2/tools/generator/internal/codegen/pipeline/unroll_recursive_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/unroll_recursive_types.go @@ -9,6 +9,7 @@ import ( "context" "strings" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -49,7 +50,7 @@ const UnrollRecursiveTypesStageID = "unrollRecursiveTypes" // a depth of 1 in practice. // If we were to unroll all loops (rather than just types that directly reference themselves like we're doing here) that // "in practice" observation may no longer hold, so we avoid doing it here (it's also more complicated to do that). -func UnrollRecursiveTypes() *Stage { +func UnrollRecursiveTypes(log logr.Logger) *Stage { return NewStage( UnrollRecursiveTypesStageID, "Unroll directly recursive types since they are not supported by controller-gen", @@ -57,7 +58,7 @@ func UnrollRecursiveTypes() *Stage { result := make(astmodel.TypeDefinitionSet) // Find object types that reference themselves - fixer := recursivetypefixer.NewSimpleRecursiveTypeFixer() + fixer := recursivetypefixer.NewSimpleRecursiveTypeFixer(log) for _, def := range state.Definitions() { updatedDef, err := fixer.Fix(def) From d99cc56f598467b75311e04b16958e451acf1966 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Wed, 24 May 2023 22:56:34 +0000 Subject: [PATCH 18/21] Fix merge --- .../internal/codegen/pipeline/report_resource_versions.go | 1 - 1 file changed, 1 deletion(-) diff --git a/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go b/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go index 1aff12ff642..b74a5e2dc14 100644 --- a/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go +++ b/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go @@ -16,7 +16,6 @@ import ( "sort" "strings" - "github.com/Azure/azure-service-operator/v2/internal/set" "github.com/pkg/errors" "golang.org/x/text/cases" "golang.org/x/text/language" From 33681c0114f951ea6fb21a0a7c01bbff0849a5c3 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 25 May 2023 01:58:28 +0000 Subject: [PATCH 19/21] Panic if validations use unsupported float values --- .../astmodel/kubebuilder_validations.go | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/v2/tools/generator/internal/astmodel/kubebuilder_validations.go b/v2/tools/generator/internal/astmodel/kubebuilder_validations.go index c53fd55c28d..fe549ba885e 100644 --- a/v2/tools/generator/internal/astmodel/kubebuilder_validations.go +++ b/v2/tools/generator/internal/astmodel/kubebuilder_validations.go @@ -157,8 +157,13 @@ func MakeMaximumValidation(value *big.Rat) KubeBuilderValidation { if value.IsInt() { return KubeBuilderValidation{MaximumValidationName, value.RatString()} } else { - floatValue, _ := value.Float64() - return KubeBuilderValidation{MaximumValidationName, floatValue} + //TODO (@bearps) Restore this check when we modify AsDeclarations() to allow error returns + if floatValue, ok := value.Float64(); ok { + return KubeBuilderValidation{MaximumValidationName, floatValue} + } + + msg := fmt.Sprintf("invalid value for maximum: %s", value.RatString()) + panic(msg) } } @@ -166,8 +171,13 @@ func MaxMinimumValidation(value *big.Rat) KubeBuilderValidation { if value.IsInt() { return KubeBuilderValidation{MinimumValidationName, value.RatString()} } else { - floatValue, _ := value.Float64() - return KubeBuilderValidation{MinimumValidationName, floatValue} + //TODO (@bearps) Restore this check when we modify AsDeclarations() to allow error returns + if floatValue, ok := value.Float64(); ok { + return KubeBuilderValidation{MinimumValidationName, floatValue} + } + + msg := fmt.Sprintf("invalid value for minimum: %s", value.RatString()) + panic(msg) } } @@ -183,7 +193,12 @@ func MakeMultipleOfValidation(value *big.Rat) KubeBuilderValidation { if value.IsInt() { return KubeBuilderValidation{MultipleOfValidationName, value.RatString()} } else { - floatValue, _ := value.Float64() - return KubeBuilderValidation{MultipleOfValidationName, floatValue} + //TODO (@bearps) Restore this check when we modify AsDeclarations() to allow error returns + if floatValue, ok := value.Float64(); ok { + return KubeBuilderValidation{MultipleOfValidationName, floatValue} + } + + msg := fmt.Sprintf("invalid value for multipleOf: %s", value.RatString()) + panic(msg) } } From 9baba9d003cef1c68ef04fbadcd203b5a33fdc0e Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 25 May 2023 05:59:33 +0000 Subject: [PATCH 20/21] Add --trace flag --- v2/tools/generator/root.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/v2/tools/generator/root.go b/v2/tools/generator/root.go index 442166f2ecb..366b216c256 100644 --- a/v2/tools/generator/root.go +++ b/v2/tools/generator/root.go @@ -46,6 +46,9 @@ func newRootCommand() (*cobra.Command, error) { rootCmd.PersistentFlags().BoolVar(&verbose, "verbose", false, "Enable verbose logging") rootCmd.PersistentFlags().BoolVar(&quiet, "quiet", false, "Suppress non-error logging") + rootCmd.PersistentFlags().BoolVar(&trace, "trace", false, "Enable trace logging (very verbose)") + + rootCmd.MarkFlagsMutuallyExclusive("verbose", "quiet", "trace") cmdFuncs := []func() (*cobra.Command, error){ NewGenTypesCommand, @@ -62,8 +65,10 @@ func newRootCommand() (*cobra.Command, error) { } rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { - // Configure logging; --verbose overrides --quiet - if verbose { + // Configure logging; --trace overrides --verbose overrides --quiet + if trace { + zerologr.SetMaxV(2) + } else if verbose { zerologr.SetMaxV(1) } else if quiet { // Can't use zerologr.SetMaxV(-1) @@ -77,8 +82,9 @@ func newRootCommand() (*cobra.Command, error) { } var ( - verbose bool quiet bool + trace bool + verbose bool ) // CreateLogger creates a logger for console output. From 04d6e761a562d0a6b8cd9015da6ba6164cdd72e3 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 25 May 2023 06:00:47 +0000 Subject: [PATCH 21/21] Add logging --- v2/tools/generator/internal/codegen/code_generator.go | 2 +- .../pipeline/implement_convertible_interface_test.go | 7 ++++--- .../pipeline/implement_convertible_spec_interface_test.go | 5 +++-- .../implement_convertible_status_interface_test.go | 5 +++-- .../pipeline/inject_property_assignment_functions.go | 8 +++++++- .../pipeline/inject_property_assignment_functions_test.go | 3 ++- .../pipeline/json_serialization_test_cases_test.go | 3 ++- .../pipeline/property_assignment_test_cases_test.go | 3 ++- .../pipeline/resource_conversion_test_cases_test.go | 3 ++- v2/tools/generator/internal/config/type_transformer.go | 2 +- 10 files changed, 27 insertions(+), 14 deletions(-) diff --git a/v2/tools/generator/internal/codegen/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index 1f59e7f9ede..2dacb3ccb6c 100644 --- a/v2/tools/generator/internal/codegen/code_generator.go +++ b/v2/tools/generator/internal/codegen/code_generator.go @@ -220,7 +220,7 @@ func createAllPipelineStages( pipeline.CreateStorageTypes().UsedFor(pipeline.ARMTarget), pipeline.CreateConversionGraph(configuration, astmodel.GeneratorVersion).UsedFor(pipeline.ARMTarget), pipeline.InjectOriginalVersionProperty().UsedFor(pipeline.ARMTarget), - pipeline.InjectPropertyAssignmentFunctions(configuration, idFactory).UsedFor(pipeline.ARMTarget), + pipeline.InjectPropertyAssignmentFunctions(configuration, idFactory, log).UsedFor(pipeline.ARMTarget), pipeline.ImplementConvertibleSpecInterface(idFactory).UsedFor(pipeline.ARMTarget), pipeline.ImplementConvertibleStatusInterface(idFactory).UsedFor(pipeline.ARMTarget), pipeline.InjectOriginalGVKFunction(idFactory).UsedFor(pipeline.ARMTarget), diff --git a/v2/tools/generator/internal/codegen/pipeline/implement_convertible_interface_test.go b/v2/tools/generator/internal/codegen/pipeline/implement_convertible_interface_test.go index b967d252df4..789507544c7 100644 --- a/v2/tools/generator/internal/codegen/pipeline/implement_convertible_interface_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/implement_convertible_interface_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -41,9 +42,9 @@ func TestGolden_InjectConvertibleInterface(t *testing.T) { cfg := config.NewConfiguration() initialState, err := RunTestPipeline( NewState().WithDefinitions(defs), - CreateStorageTypes(), // First create the storage types - CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory), // After which we inject property assignment functions + CreateStorageTypes(), // First create the storage types + CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard()), // After which we inject property assignment functions ) g.Expect(err).To(Succeed()) diff --git a/v2/tools/generator/internal/codegen/pipeline/implement_convertible_spec_interface_test.go b/v2/tools/generator/internal/codegen/pipeline/implement_convertible_spec_interface_test.go index ac8cbdb95bb..2ca3bbc57f2 100644 --- a/v2/tools/generator/internal/codegen/pipeline/implement_convertible_spec_interface_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/implement_convertible_spec_interface_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -42,8 +43,8 @@ func TestGolden_InjectConvertibleSpecInterface(t *testing.T) { initialState, CreateStorageTypes(), // First create the storage types CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory), // After which we inject property assignment functions - ImplementConvertibleSpecInterface(idFactory), // And then we get to run the stage we're testing + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard()), // After which we inject property assignment functions + ImplementConvertibleSpecInterface(idFactory), // And then we get to run the stage we're testing ) g.Expect(err).To(Succeed()) diff --git a/v2/tools/generator/internal/codegen/pipeline/implement_convertible_status_interface_test.go b/v2/tools/generator/internal/codegen/pipeline/implement_convertible_status_interface_test.go index 1c7d9b6b1bf..13ccc020e3e 100644 --- a/v2/tools/generator/internal/codegen/pipeline/implement_convertible_status_interface_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/implement_convertible_status_interface_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -42,8 +43,8 @@ func TestGolden_InjectConvertibleStatusInterface(t *testing.T) { initialState, CreateStorageTypes(), // First create the storage types CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory), // After which we inject property assignment functions - ImplementConvertibleStatusInterface(idFactory), // And then we get to run the stage we're testing + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard()), // After which we inject property assignment functions + ImplementConvertibleStatusInterface(idFactory), // And then we get to run the stage we're testing ) g.Expect(err).To(Succeed()) diff --git a/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go b/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go index faaec434909..4560ed7ea84 100644 --- a/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go +++ b/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions.go @@ -9,6 +9,7 @@ import ( "context" "github.com/dave/dst" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" @@ -27,7 +28,9 @@ const InjectPropertyAssignmentFunctionsStageID = "injectPropertyAssignmentFuncti // are the building blocks of the main CovertTo*() and ConvertFrom*() methods. func InjectPropertyAssignmentFunctions( configuration *config.Configuration, - idFactory astmodel.IdentifierFactory) *Stage { + idFactory astmodel.IdentifierFactory, + log logr.Logger, +) *Stage { stage := NewStage( InjectPropertyAssignmentFunctionsStageID, "Inject property assignment functions AssignFrom() and AssignTo() into resources and objects", @@ -40,6 +43,9 @@ func InjectPropertyAssignmentFunctions( _, ok := astmodel.AsFunctionContainer(def.Type()) if !ok { // just skip it - not a resource nor an object + log.V(2).Info( + "Skipping as no conversion functions needed", + "type", name) continue } diff --git a/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions_test.go b/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions_test.go index a609a54a5e6..4c5c639b2f4 100644 --- a/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/inject_property_assignment_functions_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -54,7 +55,7 @@ func TestGolden_InjectPropertyAssignmentFunctions(t *testing.T) { state, CreateStorageTypes(), // First create the storage types CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory)) + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard())) g.Expect(err).To(Succeed()) test.AssertPackagesGenerateExpectedCode(t, finalState.Definitions()) diff --git a/v2/tools/generator/internal/codegen/pipeline/json_serialization_test_cases_test.go b/v2/tools/generator/internal/codegen/pipeline/json_serialization_test_cases_test.go index a6c4d419174..1c106968948 100644 --- a/v2/tools/generator/internal/codegen/pipeline/json_serialization_test_cases_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/json_serialization_test_cases_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -56,7 +57,7 @@ func TestGolden_InjectJsonSerializationTests(t *testing.T) { state, CreateStorageTypes(), // First create the storage types CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory), + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard()), InjectJsonSerializationTests(idFactory)) g.Expect(err).To(Succeed()) diff --git a/v2/tools/generator/internal/codegen/pipeline/property_assignment_test_cases_test.go b/v2/tools/generator/internal/codegen/pipeline/property_assignment_test_cases_test.go index 15298c8021a..bddbbf36f16 100644 --- a/v2/tools/generator/internal/codegen/pipeline/property_assignment_test_cases_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/property_assignment_test_cases_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -53,7 +54,7 @@ func TestGolden_InjectPropertyAssignmentTests(t *testing.T) { state, CreateStorageTypes(), // First create the storage types CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory), + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard()), InjectJsonSerializationTests(idFactory), InjectPropertyAssignmentTests(idFactory)) g.Expect(err).To(Succeed()) diff --git a/v2/tools/generator/internal/codegen/pipeline/resource_conversion_test_cases_test.go b/v2/tools/generator/internal/codegen/pipeline/resource_conversion_test_cases_test.go index 47b221fe1db..e3a16118738 100644 --- a/v2/tools/generator/internal/codegen/pipeline/resource_conversion_test_cases_test.go +++ b/v2/tools/generator/internal/codegen/pipeline/resource_conversion_test_cases_test.go @@ -8,6 +8,7 @@ package pipeline import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" @@ -51,7 +52,7 @@ func TestGolden_InjectResourceConversionTestCases(t *testing.T) { NewState().WithDefinitions(defs), CreateStorageTypes(), // First create the storage types CreateConversionGraph(cfg, "v"), // Then, create the conversion graph showing relationships - InjectPropertyAssignmentFunctions(cfg, idFactory), + InjectPropertyAssignmentFunctions(cfg, idFactory, logr.Discard()), ImplementConvertibleInterface(idFactory), InjectJsonSerializationTests(idFactory), InjectPropertyAssignmentTests(idFactory), diff --git a/v2/tools/generator/internal/config/type_transformer.go b/v2/tools/generator/internal/config/type_transformer.go index 20b66cb2d2f..d4b4f81a6a6 100644 --- a/v2/tools/generator/internal/config/type_transformer.go +++ b/v2/tools/generator/internal/config/type_transformer.go @@ -345,7 +345,7 @@ func (r PropertyTransformResult) LogTo(log logr.Logger) { return } - log.V(1).Info( + log.V(2).Info( "Transforming property", "type", r.TypeName, "property", r.Property,