From a433d2e8341facc4d5b236417620683d429bb1d1 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Thu, 28 Mar 2024 20:57:55 +0100 Subject: [PATCH] fix errcheck linter Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- .golangci.yaml | 1 - pkg/factory/factory.go | 4 +- pkg/inspect/secret/secret.go | 93 ++++++++++++++++++++------- pkg/inspect/secret/secret_test.go | 36 +++++++++-- pkg/install/helm/flags.go | 8 ++- pkg/install/helm/settings.go | 4 +- pkg/install/install.go | 8 ++- pkg/install/util.go | 24 +++++-- test/integration/ctl_install_test.go | 4 +- test/integration/framework/helpers.go | 16 +++-- 10 files changed, 148 insertions(+), 50 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index b0b60c9..8919557 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,7 +1,6 @@ issues: exclude-rules: - linters: - - errcheck - gosec text: ".*" linters: diff --git a/pkg/factory/factory.go b/pkg/factory/factory.go index ef87ce8..10f7dfa 100644 --- a/pkg/factory/factory.go +++ b/pkg/factory/factory.go @@ -70,7 +70,9 @@ func New(cmd *cobra.Command) *Factory { f.factory = util.NewFactory(kubeConfigFlags) kubeConfigFlags.AddFlags(cmd.Flags()) - cmd.RegisterFlagCompletionFunc("namespace", validArgsListNamespaces(f)) + if err := cmd.RegisterFlagCompletionFunc("namespace", validArgsListNamespaces(f)); err != nil { + panic(err) + } // Setup a PreRunE to populate the Factory. Catch the existing PreRunE command // if one was defined, and execute it second. diff --git a/pkg/inspect/secret/secret.go b/pkg/inspect/secret/secret.go index 4d76994..ac99988 100644 --- a/pkg/inspect/secret/secret.go +++ b/pkg/inspect/secret/secret.go @@ -165,13 +165,26 @@ func (o *Options) Run(ctx context.Context, args []string, stdout io.Writer) erro return fmt.Errorf("error when parsing 'tls.crt': %w", err) } - out := []string{ - describeValidFor(x509Cert), - describeValidityPeriod(x509Cert), - describeIssuedBy(x509Cert), - describeIssuedFor(x509Cert), - describeCertificate(x509Cert), - describeDebugging(ctx, x509Cert, intermediates, secret.Data[cmmeta.TLSCAKey]), + var out []string + + for _, describeFn := range []func(*x509.Certificate) (string, error){ + describeValidFor, + describeValidityPeriod, + describeIssuedBy, + describeIssuedFor, + describeCertificate, + } { + desc, err := describeFn(x509Cert) + if err != nil { + return err + } + out = append(out, desc) + } + + if desc, err := describeDebugging(ctx, x509Cert, intermediates, secret.Data[cmmeta.TLSCAKey]); err != nil { + return err + } else { + out = append(out, desc) } fmt.Fprintln(stdout, strings.Join(out, "\n\n")) @@ -179,9 +192,14 @@ func (o *Options) Run(ctx context.Context, args []string, stdout io.Writer) erro return nil } -func describeValidFor(cert *x509.Certificate) string { +func describeValidFor(cert *x509.Certificate) (string, error) { + tmpl, err := template.New("validForTemplate").Parse(validForTemplate) + if err != nil { + return "", err + } + var b bytes.Buffer - template.Must(template.New("validForTemplate").Parse(validForTemplate)).Execute(&b, struct { + err = tmpl.Execute(&b, struct { DNSNames string URIs string IPAddresses string @@ -195,12 +213,17 @@ func describeValidFor(cert *x509.Certificate) string { KeyUsage: printKeyUsage(pki.BuildCertManagerKeyUsages(cert.KeyUsage, cert.ExtKeyUsage)), }) - return b.String() + return b.String(), err } -func describeValidityPeriod(cert *x509.Certificate) string { +func describeValidityPeriod(cert *x509.Certificate) (string, error) { + tmpl, err := template.New("validityPeriodTemplate").Parse(validityPeriodTemplate) + if err != nil { + return "", err + } + var b bytes.Buffer - template.Must(template.New("validityPeriodTemplate").Parse(validityPeriodTemplate)).Execute(&b, struct { + err = tmpl.Execute(&b, struct { NotBefore string NotAfter string }{ @@ -208,12 +231,17 @@ func describeValidityPeriod(cert *x509.Certificate) string { NotAfter: cert.NotAfter.Format(time.RFC1123), }) - return b.String() + return b.String(), err } -func describeIssuedBy(cert *x509.Certificate) string { +func describeIssuedBy(cert *x509.Certificate) (string, error) { + tmpl, err := template.New("issuedByTemplate").Parse(issuedByTemplate) + if err != nil { + return "", err + } + var b bytes.Buffer - template.Must(template.New("issuedByTemplate").Parse(issuedByTemplate)).Execute(&b, struct { + err = tmpl.Execute(&b, struct { CommonName string Organization string OrganizationalUnit string @@ -225,12 +253,17 @@ func describeIssuedBy(cert *x509.Certificate) string { Country: printSliceOrOne(cert.Issuer.Country), }) - return b.String() + return b.String(), err } -func describeIssuedFor(cert *x509.Certificate) string { +func describeIssuedFor(cert *x509.Certificate) (string, error) { + tmpl, err := template.New("issuedForTemplate").Parse(issuedForTemplate) + if err != nil { + return "", err + } + var b bytes.Buffer - template.Must(template.New("issuedForTemplate").Parse(issuedForTemplate)).Execute(&b, struct { + err = tmpl.Execute(&b, struct { CommonName string Organization string OrganizationalUnit string @@ -242,12 +275,17 @@ func describeIssuedFor(cert *x509.Certificate) string { Country: printSliceOrOne(cert.Subject.Country), }) - return b.String() + return b.String(), err } -func describeCertificate(cert *x509.Certificate) string { +func describeCertificate(cert *x509.Certificate) (string, error) { + tmpl, err := template.New("certificateTemplate").Parse(certificateTemplate) + if err != nil { + return "", err + } + var b bytes.Buffer - template.Must(template.New("certificateTemplate").Parse(certificateTemplate)).Execute(&b, struct { + err = tmpl.Execute(&b, struct { SigningAlgorithm string PublicKeyAlgorithm string SerialNumber string @@ -265,12 +303,17 @@ func describeCertificate(cert *x509.Certificate) string { OCSP: printSliceOrOne(cert.OCSPServer), }) - return b.String() + return b.String(), err } -func describeDebugging(ctx context.Context, cert *x509.Certificate, intermediates [][]byte, ca []byte) string { +func describeDebugging(ctx context.Context, cert *x509.Certificate, intermediates [][]byte, ca []byte) (string, error) { + tmpl, err := template.New("debuggingTemplate").Parse(debuggingTemplate) + if err != nil { + return "", err + } + var b bytes.Buffer - template.Must(template.New("debuggingTemplate").Parse(debuggingTemplate)).Execute(&b, struct { + err = tmpl.Execute(&b, struct { TrustedByThisComputer string CRLStatus string OCSPStatus string @@ -280,7 +323,7 @@ func describeDebugging(ctx context.Context, cert *x509.Certificate, intermediate OCSPStatus: describeOCSP(ctx, cert, intermediates, ca), }) - return b.String() + return b.String(), err } func describeCRL(ctx context.Context, cert *x509.Certificate) string { diff --git a/pkg/inspect/secret/secret_test.go b/pkg/inspect/secret/secret_test.go index cbaba89..43978a3 100644 --- a/pkg/inspect/secret/secret_test.go +++ b/pkg/inspect/secret/secret_test.go @@ -167,9 +167,13 @@ func Test_describeCertificate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := describeCertificate(tt.cert); got != tt.want { + got, err := describeCertificate(tt.cert) + if got != tt.want { t.Errorf("describeCertificate() = %v, want %v", makeInvisibleVisible(got), makeInvisibleVisible(tt.want)) } + if err != nil { + t.Errorf("describeCertificate() error = %v", err) + } }) } } @@ -201,9 +205,13 @@ func Test_describeDebugging(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := describeDebugging(context.TODO(), tt.args.cert, tt.args.intermediates, tt.args.ca); got != tt.want { + got, err := describeDebugging(context.TODO(), tt.args.cert, tt.args.intermediates, tt.args.ca) + if got != tt.want { t.Errorf("describeDebugging() = %v, want %v", makeInvisibleVisible(got), makeInvisibleVisible(tt.want)) } + if err != nil { + t.Errorf("describeCertificate() error = %v", err) + } }) } } @@ -226,9 +234,13 @@ func Test_describeIssuedBy(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := describeIssuedBy(tt.cert); got != tt.want { + got, err := describeIssuedBy(tt.cert) + if got != tt.want { t.Errorf("describeIssuedBy() = %v, want %v", makeInvisibleVisible(got), makeInvisibleVisible(tt.want)) } + if err != nil { + t.Errorf("describeIssuedBy() error = %v", err) + } }) } } @@ -251,9 +263,13 @@ func Test_describeIssuedFor(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := describeIssuedFor(tt.cert); got != tt.want { + got, err := describeIssuedFor(tt.cert) + if got != tt.want { t.Errorf("describeIssuedFor() = %v, want %v", makeInvisibleVisible(got), makeInvisibleVisible(tt.want)) } + if err != nil { + t.Errorf("describeCertificate() error = %v", err) + } }) } } @@ -349,9 +365,13 @@ func Test_describeValidFor(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := describeValidFor(tt.cert); got != tt.want { + got, err := describeValidFor(tt.cert) + if got != tt.want { t.Errorf("describeValidFor() = %v, want %v", makeInvisibleVisible(got), makeInvisibleVisible(tt.want)) } + if err != nil { + t.Errorf("describeIssuedBy() error = %v", err) + } }) } } @@ -372,9 +392,13 @@ func Test_describeValidityPeriod(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := describeValidityPeriod(tt.cert); got != tt.want { + got, err := describeValidityPeriod(tt.cert) + if got != tt.want { t.Errorf("describeValidityPeriod() = %v, want %v", makeInvisibleVisible(got), makeInvisibleVisible(tt.want)) } + if err != nil { + t.Errorf("describeValidityPeriod() error = %v", err) + } }) } } diff --git a/pkg/install/helm/flags.go b/pkg/install/helm/flags.go index 42157d0..10fe9f8 100644 --- a/pkg/install/helm/flags.go +++ b/pkg/install/helm/flags.go @@ -25,7 +25,11 @@ import ( // Flags that are shared between the Install and the Uninstall command func AddInstallUninstallFlags(f *pflag.FlagSet, timeout *time.Duration, wait *bool) { f.DurationVar(timeout, "timeout", 300*time.Second, "Time to wait for any individual Kubernetes operation (like Jobs for hooks)") - f.MarkHidden("timeout") + if err := f.MarkHidden("timeout"); err != nil { + panic(err) + } f.BoolVar(wait, "wait", true, "If set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment, StatefulSet, or ReplicaSet are in a ready state before marking the release as successful. It will wait for as long as --timeout") - f.MarkHidden("wait") + if err := f.MarkHidden("wait"); err != nil { + panic(err) + } } diff --git a/pkg/install/helm/settings.go b/pkg/install/helm/settings.go index 99c85a9..076357b 100644 --- a/pkg/install/helm/settings.go +++ b/pkg/install/helm/settings.go @@ -74,7 +74,9 @@ func (n *NormalisedEnvSettings) Setup(ctx context.Context, cmd *cobra.Command) { // Fix the default namespace to be cert-manager cmd.Flag("namespace").DefValue = defaultCertManagerNamespace - cmd.Flag("namespace").Value.Set(defaultCertManagerNamespace) + if err := cmd.Flag("namespace").Value.Set(defaultCertManagerNamespace); err != nil { + panic(err) + } } func (n *NormalisedEnvSettings) setupEnvSettings(cmd *cobra.Command) { diff --git a/pkg/install/install.go b/pkg/install/install.go index be1bad5..1ecf116 100644 --- a/pkg/install/install.go +++ b/pkg/install/install.go @@ -117,9 +117,13 @@ func NewCmdInstall(setupCtx context.Context, ioStreams genericclioptions.IOStrea addChartPathOptionsFlags(cmd.Flags(), &options.client.ChartPathOptions) cmd.Flags().BoolVar(&options.client.CreateNamespace, "create-namespace", true, "Create the release namespace if not present") - cmd.Flags().MarkHidden("create-namespace") + if err := cmd.Flags().MarkHidden("create-namespace"); err != nil { + panic(err) + } cmd.Flags().StringVar(&options.ChartName, "chart-name", "cert-manager", "Name of the chart to install") - cmd.Flags().MarkHidden("chart-name") + if err := cmd.Flags().MarkHidden("chart-name"); err != nil { + panic(err) + } cmd.Flags().BoolVar(&options.DryRun, "dry-run", false, "Simulate install and output manifest") return cmd diff --git a/pkg/install/util.go b/pkg/install/util.go index 4b8dd1b..957db5c 100644 --- a/pkg/install/util.go +++ b/pkg/install/util.go @@ -28,22 +28,34 @@ import ( func addInstallFlags(f *pflag.FlagSet, client *action.Install) { f.StringVar(&client.ReleaseName, "release-name", "cert-manager", "Name of the helm release") - f.MarkHidden("release-name") + if err := f.MarkHidden("release-name"); err != nil { + panic(err) + } f.BoolVarP(&client.GenerateName, "generate-name", "g", false, "Generate the name (instead of using the default 'cert-manager' value)") - f.MarkHidden("generate-name") + if err := f.MarkHidden("generate-name"); err != nil { + panic(err) + } f.StringVar(&client.NameTemplate, "name-template", "", "Specify template used to name the release") - f.MarkHidden("name-template") + if err := f.MarkHidden("name-template"); err != nil { + panic(err) + } f.StringVar(&client.Description, "description", "cert-manager was installed using the cert-manager CLI", "Add a custom description") - f.MarkHidden("description") + if err := f.MarkHidden("description"); err != nil { + panic(err) + } } func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { f.StringSliceVarP(&v.ValueFiles, "values", "f", []string{}, "Specify values in a YAML file or a URL (can specify multiple)") f.StringArrayVar(&v.Values, "set", []string{}, "Set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.StringValues, "set-string", []string{}, "Set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") - f.MarkHidden("set-string") + if err := f.MarkHidden("set-string"); err != nil { + panic(err) + } f.StringArrayVar(&v.FileValues, "set-file", []string{}, "Set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)") - f.MarkHidden("set-file") + if err := f.MarkHidden("set-file"); err != nil { + panic(err) + } } // defaultKeyring returns the expanded path to the default keyring. diff --git a/test/integration/ctl_install_test.go b/test/integration/ctl_install_test.go index e7aef91..2024abb 100644 --- a/test/integration/ctl_install_test.go +++ b/test/integration/ctl_install_test.go @@ -148,7 +148,9 @@ func executeCmctlAndCheckOutput( expErr bool, expOutput string, ) { - logsapi.ResetForTest(nil) + if err := logsapi.ResetForTest(nil); err != nil { + t.Fatal(err) + } executeAndCheckOutput(t, func(stdin io.Reader, stdout io.Writer) error { cmd := cmd.NewCertManagerCtlCommand(ctx, stdin, stdout, stdout) diff --git a/test/integration/framework/helpers.go b/test/integration/framework/helpers.go index 4da22d8..d214da1 100644 --- a/test/integration/framework/helpers.go +++ b/test/integration/framework/helpers.go @@ -65,11 +65,17 @@ func NewClients(t *testing.T, config *rest.Config) (kubernetes.Interface, cmclie cmFactory := cminformers.NewSharedInformerFactory(cmCl, 0) scheme := runtime.NewScheme() - kscheme.AddToScheme(scheme) - certmgrscheme.AddToScheme(scheme) - apiext.AddToScheme(scheme) - apireg.AddToScheme(scheme) - gwapi.AddToScheme(scheme) + for _, err := range []error{ + kscheme.AddToScheme(scheme), + certmgrscheme.AddToScheme(scheme), + apiext.AddToScheme(scheme), + apireg.AddToScheme(scheme), + gwapi.AddToScheme(scheme), + } { + if err != nil { + t.Fatal(err) + } + } return cl, cmCl, cmFactory, scheme }