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 ffdb2565e5f..e7a175a1658 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() - cg, err := codegen.NewCodeGeneratorFromConfigFile(configFile) + log := CreateLogger() + + cg, err := codegen.NewCodeGeneratorFromConfigFile(configFile, log) if err != nil { - klog.Errorf("Error creating code generator: %s\n", err) + log.Error(err, "Error creating code generator") return err } @@ -42,22 +43,27 @@ 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) + log.Error(err, "Error generating code generation") + return err } return nil 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= 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..fe853fb9d46 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,22 +129,26 @@ func (e *ErroredType) RequiredPackageReferences() *PackageReferenceSet { } func (e *ErroredType) handleWarningsAndErrors() { - for _, warning := range e.warnings { - klog.Warning(warning) - } + var errs []error 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)) + // 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(errs) == 1 { + panic(errs[0]) + } else { + panic(kerrors.NewAggregate(errs)) + } } func (e *ErroredType) AsDeclarations(cgc *CodeGenerationContext, dc DeclarationContext) []dst.Decl { diff --git a/v2/tools/generator/internal/astmodel/kubebuilder_validations.go b/v2/tools/generator/internal/astmodel/kubebuilder_validations.go index d4712e997c3..fe549ba885e 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,12 +157,13 @@ 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) + //TODO (@bearps) Restore this check when we modify AsDeclarations() to allow error returns + if floatValue, ok := value.Float64(); ok { + return KubeBuilderValidation{MaximumValidationName, floatValue} } - return KubeBuilderValidation{MaximumValidationName, floatValue} + msg := fmt.Sprintf("invalid value for maximum: %s", value.RatString()) + panic(msg) } } @@ -171,12 +171,13 @@ 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) + //TODO (@bearps) Restore this check when we modify AsDeclarations() to allow error returns + if floatValue, ok := value.Float64(); ok { + return KubeBuilderValidation{MinimumValidationName, floatValue} } - return KubeBuilderValidation{MinimumValidationName, floatValue} + msg := fmt.Sprintf("invalid value for minimum: %s", value.RatString()) + panic(msg) } } @@ -192,11 +193,12 @@ 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) + //TODO (@bearps) Restore this check when we modify AsDeclarations() to allow error returns + if floatValue, ok := value.Float64(); ok { + return KubeBuilderValidation{MultipleOfValidationName, floatValue} } - return KubeBuilderValidation{MultipleOfValidationName, floatValue} + msg := fmt.Sprintf("invalid value for multipleOf: %s", value.RatString()) + panic(msg) } } 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/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/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/code_generator.go b/v2/tools/generator/internal/codegen/code_generator.go index c6db7edea0b..2dacb3ccb6c 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,22 +77,34 @@ 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), + pipeline.LoadTypes(idFactory, configuration, log), // Assemble actual one-of types from roots and leaves pipeline.AssembleOneOfTypes(idFactory), @@ -108,12 +129,12 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration // 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(), - pipeline.UnrollRecursiveTypes(), + pipeline.UnrollRecursiveTypes(log), pipeline.RemoveStatusValidations(), // Figure out resource owners: @@ -125,11 +146,11 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration 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(), @@ -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(), @@ -151,7 +172,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration 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), @@ -166,14 +187,14 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration 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(), + pipeline.FlattenProperties(log), // Remove types which may not be needed after flattening pipeline.StripUnreferencedTypeDefinitions(), @@ -199,7 +220,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration 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), @@ -228,7 +249,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration 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), @@ -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,70 +281,88 @@ 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()) - + 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()) + return errors.Wrapf(err, "failed to execute stage %d: %s", stageNumber, 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()) - } + duration := time.Since(start).Round(time.Millisecond) - generator.logStateChange(state, newState) + defsAdded := newState.Definitions().Except(state.Definitions()) + defsRemoved := state.Definitions().Except(newState.Definitions()) - 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()) - } - } - - 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, + "added", len(defsAdded), + "removed", len(defsRemoved)) state = newState } if err := state.CheckFinalState(); err != nil { - klog.Info("Failed") return err } - klog.Info("Finished") + log.Info("Finished") return nil } -func (generator *CodeGenerator) logStateChange(former *pipeline.State, later *pipeline.State) { - defsAdded := later.Definitions().Except(former.Definitions()) - defsRemoved := former.Definitions().Except(later.Definitions()) +// 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: %s", 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") + } - 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)) + // 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. 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..17cfe749a68 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/remover.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/remover.go @@ -8,9 +8,9 @@ package embeddedresources import ( "fmt" + "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" @@ -103,7 +103,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) @@ -130,12 +132,12 @@ func (e EmbeddedResourceRemover) RemoveEmbeddedResources() (astmodel.TypeDefinit } } - result, err := simplifyTypeNames(result, e.typeFlag, originalNames) + result, err := simplifyTypeNames(result, e.typeFlag, originalNames, log) if err != nil { return nil, err } - return RemoveEmptyObjects(result) + return RemoveEmptyObjects(result, log) } func (e EmbeddedResourceRemover) makeEmbeddedResourceRemovalTypeVisitor() astmodel.TypeVisitor { @@ -242,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 } @@ -267,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..9b89320cf36 100644 --- a/v2/tools/generator/internal/codegen/embeddedresources/renamer.go +++ b/v2/tools/generator/internal/codegen/embeddedresources/renamer.go @@ -8,8 +8,8 @@ package embeddedresources import ( "fmt" + "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -22,22 +22,28 @@ 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 { 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 + log.V(2).Info( + "Type not used, renaming for simplicity", + "originalName", associatedNames.Single(), + "newName", original) + return renames, nil } @@ -45,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 @@ -61,7 +68,11 @@ 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]) + + log.V(2).Info( + "Type used in single context, renaming for simplicity", + "originalName", associated, + "newName", renames[associated]) return renames, nil } @@ -70,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)) @@ -94,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 @@ -103,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 @@ -159,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) @@ -188,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/golden_files_test.go b/v2/tools/generator/internal/codegen/golden_files_test.go index 9c5be7f799e..ce004eac68b 100644 --- a/v2/tools/generator/internal/codegen/golden_files_test.go +++ b/v2/tools/generator/internal/codegen/golden_files_test.go @@ -9,17 +9,16 @@ import ( "bytes" "context" "fmt" - "io/ioutil" "os" "path/filepath" "strings" "testing" + "github.com/go-logr/logr" "github.com/pkg/errors" "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" @@ -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) { @@ -123,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) } @@ -147,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 } @@ -228,9 +227,7 @@ 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) + inputFile, err := os.ReadFile(path) if err != nil { return nil, errors.Wrapf(err, "cannot read golden test input file") } @@ -241,9 +238,7 @@ func loadTestSchemaIntoTypes( return nil, errors.Wrapf(err, "could not compile input") } - scanner := jsonast.NewSchemaScanner(idFactory, configuration) - - klog.V(0).Infof("Walking deployment template") + scanner := jsonast.NewSchemaScanner(idFactory, configuration, logr.Discard()) schemaAbstraction := jsonast.MakeGoJSONSchema(schema.Root(), configuration.MakeLocalPackageReference, idFactory) _, err = scanner.GenerateAllDefinitions(ctx, schemaAbstraction) @@ -349,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/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..742c2f33071 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.LogTo(log) objectType = transformation.NewType } 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/convert_allof_and_oneof_to_objects.go b/v2/tools/generator/internal/codegen/pipeline/convert_allof_and_oneof_to_objects.go index 48430ebddb1..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 @@ -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 // continue } 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..70d42630568 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.V(1).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..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 @@ -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" @@ -43,9 +44,9 @@ func TestCreateFlattenedARMType_CreatesExpectedConversions(t *testing.T) { idFactory := astmodel.NewIdentifierFactory() - createARMTypes := CreateARMTypes(idFactory) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) - flatten := FlattenProperties() + flatten := FlattenProperties(logr.Discard()) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() @@ -97,11 +98,11 @@ 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) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) - flatten := FlattenProperties() + flatten := FlattenProperties(logr.Discard()) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() @@ -166,9 +167,9 @@ func TestCreateFlattenedARMTypeWithConfigMap_CreatesExpectedConversions(t *testi configuration.ObjectModelConfiguration = omc addConfigMaps := AddConfigMaps(configuration) - createARMTypes := CreateARMTypes(idFactory) + createARMTypes := CreateARMTypes(idFactory, logr.Discard()) applyARMConversionInterface := ApplyARMConversionInterface(idFactory) - flatten := FlattenProperties() + flatten := FlattenProperties(logr.Discard()) simplify := SimplifyDefinitions() strip := StripUnreferencedTypeDefinitions() @@ -242,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/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/flatten_properties.go b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go index 0e6a14d4608..e21ae89920a 100644 --- a/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go +++ b/v2/tools/generator/internal/codegen/pipeline/flatten_properties.go @@ -9,36 +9,37 @@ import ( "context" "strings" + "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/klog/v2" "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) @@ -50,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 } @@ -58,10 +59,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 @@ -144,6 +141,7 @@ func collectAndFlattenProperties( container astmodel.TypeName, objectType *astmodel.ObjectType, defs astmodel.TypeDefinitionSet, + log logr.Logger, ) ([]*astmodel.PropertyDefinition, error) { var flattenedProps []*astmodel.PropertyDefinition @@ -154,9 +152,14 @@ func collectAndFlattenProperties( return // continue } - innerProps, err := flattenProperty(container, prop, defs) + innerProps, err := flattenProperty(container, prop, defs, logr.Discard()) if err != nil { - klog.Warningf("Skipping flatten of %s on %s: %s", prop.PropertyName(), container, err) + log.V(2).Info( + "Skipping flatten", + "property", prop.PropertyName(), + "container", container, + "reason", err) + innerProps = []*astmodel.PropertyDefinition{ prop.SetFlatten(false), } @@ -172,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()) } @@ -190,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: @@ -203,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 } @@ -212,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..075768555b0 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,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) + 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()) @@ -46,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) { @@ -67,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) + 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/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 d6c6061a95e..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,8 +9,8 @@ import ( "context" "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" @@ -28,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", @@ -41,12 +43,12 @@ 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) + log.V(2).Info( + "Skipping as no conversion functions needed", + "type", 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_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/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/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/load_types.go b/v2/tools/generator/internal/codegen/pipeline/load_types.go index 22a09fc67c8..6eb31d209a7 100644 --- a/v2/tools/generator/internal/codegen/pipeline/load_types.go +++ b/v2/tools/generator/internal/codegen/pipeline/load_types.go @@ -8,19 +8,19 @@ package pipeline import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "regexp" "sort" "strings" "sync" + "time" + "github.com/go-logr/logr" "github.com/go-openapi/spec" "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" @@ -37,19 +37,28 @@ 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) + log.V(1).Info( + "Loading Swagger data", + "source", 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") } - 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") @@ -111,8 +120,6 @@ func LoadTypes(idFactory astmodel.IdentifierFactory, config *config.Configuratio } } - klog.V(1).Infof("Input %d definitions, output %d definitions", len(definitions), len(defs)) - return defs, nil }) } @@ -292,8 +299,19 @@ 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) { - schemas, err := loadAllSchemas(ctx, config.SchemaRoot, config.LocalPathPrefix(), idFactory, config.Status.Overrides) +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, + log) if err != nil { return jsonast.SwaggerTypes{}, err } @@ -301,7 +319,14 @@ func loadSwaggerData(ctx context.Context, idFactory astmodel.IdentifierFactory, 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, @@ -309,7 +334,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 { @@ -319,19 +345,38 @@ func loadSwaggerData(ctx context.Context, idFactory astmodel.IdentifierFactory, typesByGroup[*schema.Package] = append(typesByGroup[*schema.Package], typesFromFile{types, schemaPath}) } + log.Info( + "Loaded Swagger files", + "loaded", countLoaded, + "total", len(schemas)) + return mergeSwaggerTypesByGroup(idFactory, typesByGroup) } -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)) +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), 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 { @@ -392,8 +437,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 { @@ -625,11 +668,16 @@ 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) + 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 @@ -659,12 +707,20 @@ 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 - klog.V(3).Infof("Loading file %q", filePath) + log.V(1).Info( + "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) } @@ -686,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 @@ -708,14 +767,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..0f05522c9ef 100644 --- a/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go +++ b/v2/tools/generator/internal/codegen/pipeline/pipeline_diagram.go @@ -7,13 +7,12 @@ package pipeline import ( "fmt" - "io/ioutil" + "os" "path/filepath" "strings" "unicode" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astbuilder" ) @@ -37,8 +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) - klog.V(2).Infof("Wrote diagram for pipeline to %s", filename) + err := os.WriteFile(filename, dotsrc, 0600) return errors.Wrapf(err, "failed to write diagram to %s", filename) } 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/recursivetypefixer/simple_recursive_type_fixer.go b/v2/tools/generator/internal/codegen/pipeline/recursivetypefixer/simple_recursive_type_fixer.go index b6c331befea..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 @@ -6,10 +6,10 @@ package recursivetypefixer import ( + "github.com/Azure/azure-service-operator/v2/internal/set" + "github.com/go-logr/logr" "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" ) @@ -19,6 +19,7 @@ import ( type SimpleRecursiveTypeFixer struct { visitor *astmodel.TypeVisitor newDefinitions astmodel.TypeDefinitionSet + log logr.Logger } type simpleRecursiveTypeFixerContext struct { @@ -33,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{ @@ -102,7 +104,11 @@ 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) + 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/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/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 a0260c08b9b..b74a5e2dc14 100644 --- a/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go +++ b/v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go @@ -20,7 +20,6 @@ import ( "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/internal/util/typo" @@ -182,8 +181,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 @@ -216,8 +213,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 @@ -603,7 +598,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/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/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/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) 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 diff --git a/v2/tools/generator/internal/config/type_transformer.go b/v2/tools/generator/internal/config/type_transformer.go index 4db80dd77be..d4b4f81a6a6 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 { ) } +// LogTo creates a log message for the transformation +func (r PropertyTransformResult) LogTo(log logr.Logger) { + if r.Removed { + log.V(2).Info( + "Removing property", + "type", r.TypeName, + "property", r.Property, + "because", r.Because) + return + } + + log.V(2).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/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/openapi_file_loader.go b/v2/tools/generator/internal/jsonast/openapi_file_loader.go index f97a14e618c..20fd28c06a3 100644 --- a/v2/tools/generator/internal/jsonast/openapi_file_loader.go +++ b/v2/tools/generator/internal/jsonast/openapi_file_loader.go @@ -7,12 +7,11 @@ package jsonast import ( "fmt" - "io/ioutil" + "os" "path/filepath" "github.com/go-openapi/spec" "github.com/pkg/errors" - "k8s.io/klog/v2" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" ) @@ -44,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 { @@ -71,9 +72,7 @@ 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) + fileContent, err := os.ReadFile(absPath) if err != nil { return result, errors.Wrapf(err, "unable to read swagger file %q", absPath) } 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..143fcabfab1 100644 --- a/v2/tools/generator/internal/jsonast/swagger_type_extractor.go +++ b/v2/tools/generator/internal/jsonast/swagger_type_extractor.go @@ -8,13 +8,15 @@ package jsonast import ( "context" "fmt" + "os" + "path/filepath" "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 +24,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 +41,7 @@ func NewSwaggerTypeExtractor( swaggerPath string, outputPackage astmodel.LocalPackageReference, cache CachingFileLoader, + log logr.Logger, ) SwaggerTypeExtractor { return SwaggerTypeExtractor{ idFactory: idFactory, @@ -47,6 +50,7 @@ func NewSwaggerTypeExtractor( outputPackage: outputPackage, cache: cache, config: config, + log: log, } } @@ -76,7 +80,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 { @@ -120,7 +124,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 } @@ -156,17 +159,32 @@ 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 { - klog.Errorf("Error extracting resource name (%s): %s", extractor.swaggerPath, err.Error()) + // 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, osErr := os.Getwd(); osErr == nil { + if d, pathErr := filepath.Rel(cwd, extractor.swaggerPath); pathErr == nil { + dir = d + } + } + + extractor.log.V(1).Info( + "Error extracting resource name", + "swaggerPath", dir, + "error", err) 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 +254,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 +277,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 +336,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 +367,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(1).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.V(1).Info( + "no schema found for PUT operation", + "operation", rawOperationPath, + "swagger", extractor.swaggerPath) return nil, nil, false } } @@ -377,7 +404,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 +413,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 +422,8 @@ func (extractor *SwaggerTypeExtractor) schemaFromParameter(param spec.Parameter) paramPath, extractor.outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) } return &result @@ -412,7 +440,8 @@ func (extractor *SwaggerTypeExtractor) doesResponseRepresentARMResource(response extractor.swaggerPath, extractor.outputPackage, extractor.idFactory, - extractor.cache) + extractor.cache, + extractor.log) return &result, isMarkedAsARMResource(result) } @@ -432,12 +461,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.V(1).Info( + "no schema found for response", + "operation", rawOperationPath, + "swagger", extractor.swaggerPath) return nil, false } @@ -582,7 +615,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 } 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(), 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 } 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() diff --git a/v2/tools/generator/root.go b/v2/tools/generator/root.go index e96b40eb5e9..366b216c256 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,18 @@ 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") + rootCmd.PersistentFlags().BoolVar(&trace, "trace", false, "Enable trace logging (very verbose)") + + rootCmd.MarkFlagsMutuallyExclusive("verbose", "quiet", "trace") + cmdFuncs := []func() (*cobra.Command, error){ NewGenTypesCommand, NewGenKustomizeCommand, @@ -50,5 +64,46 @@ func newRootCommand() (*cobra.Command, error) { rootCmd.AddCommand(cmd) } + rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + // 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) + zerolog.SetGlobalLevel(zerolog.ErrorLevel) + } else { + zerologr.SetMaxV(0) + } + } + return rootCmd, nil } + +var ( + quiet bool + trace bool + verbose bool +) + +// CreateLogger creates a logger for console output. +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 +}