From 95411c7db189f31e9a5ab8e35b9e404f35156689 Mon Sep 17 00:00:00 2001 From: Ricardo Maraschini Date: Fri, 11 Oct 2024 14:30:25 +0200 Subject: [PATCH 1/2] bug: stop fetching support bundle uri on airgap the function `ParseSupportBundleFromDoc()` always attempt to fetch the support bundle from its uri (if present). there is another function that allows us to define if we want or not use the uri: ``` ParseSupportBundle() ``` this new function requires a second argument called `followURI` and we need to set it to false when kots is running in an airgap environment. --- pkg/supportbundle/defaultspec/embed.go | 17 ++++++++--------- pkg/supportbundle/spec.go | 22 ++++++++++++---------- pkg/supportbundle/staticspecs/embed.go | 4 ++-- pkg/supportbundle/supportbundle.go | 7 ++++++- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/pkg/supportbundle/defaultspec/embed.go b/pkg/supportbundle/defaultspec/embed.go index e420b2efc9..b41dff158a 100644 --- a/pkg/supportbundle/defaultspec/embed.go +++ b/pkg/supportbundle/defaultspec/embed.go @@ -12,14 +12,13 @@ var raw []byte var spec *troubleshootv1beta2.SupportBundle -func init() { - var err error - spec, err = supportbundle.ParseSupportBundleFromDoc(raw) - if err != nil { - panic(err) +func Get(isAirgap bool) (troubleshootv1beta2.SupportBundle, error) { + if spec == nil { + var err error + spec, err = supportbundle.ParseSupportBundle(raw, !isAirgap) + if err != nil { + return troubleshootv1beta2.SupportBundle{}, err + } } -} - -func Get() troubleshootv1beta2.SupportBundle { - return *spec.DeepCopy() + return *spec.DeepCopy(), nil } diff --git a/pkg/supportbundle/spec.go b/pkg/supportbundle/spec.go index 887215522b..cf127a6a34 100644 --- a/pkg/supportbundle/spec.go +++ b/pkg/supportbundle/spec.go @@ -106,7 +106,7 @@ func CreateRenderedSpec(app *apptypes.App, sequence int64, kotsKinds *kotsutil.K } // split the default kotsadm support bundle into multiple support bundles - vendorSpec, err := createVendorSpec(builtBundle) + vendorSpec, err := createVendorSpec(builtBundle, app.IsAirgap) if err != nil { return nil, errors.Wrap(err, "failed to create vendor support bundle spec") } @@ -370,8 +370,8 @@ func addAfterCollectionSpec(app *apptypes.App, b *troubleshootv1beta2.SupportBun } // createVendorSpec creates a support bundle spec that includes the vendor specific collectors and analyzers -func createVendorSpec(b *troubleshootv1beta2.SupportBundle) (*troubleshootv1beta2.SupportBundle, error) { - supportBundle, err := staticspecs.GetVendorSpec() +func createVendorSpec(b *troubleshootv1beta2.SupportBundle, isAirgap bool) (*troubleshootv1beta2.SupportBundle, error) { + supportBundle, err := staticspecs.GetVendorSpec(isAirgap) if err != nil { logger.Errorf("Failed to load vendor support bundle spec: %v", err) return nil, err @@ -457,15 +457,12 @@ func addDiscoveredSpecs( } for _, specData := range specs { - sbObject, err := sb.ParseSupportBundleFromDoc([]byte(specData)) + sbObject, err := sb.ParseSupportBundle([]byte(specData), !app.IsAirgap) if err != nil { logger.Errorf("Failed to unmarshal support bundle spec: %v", err) continue } - // ParseSupportBundleFromDoc will check if there is a uri field and if so, - // use the upstream spec, otherwise fall back to - // what's defined in the current spec supportBundle.Spec.Collectors = append(supportBundle.Spec.Collectors, sbObject.Spec.Collectors...) supportBundle.Spec.Analyzers = append(supportBundle.Spec.Analyzers, sbObject.Spec.Analyzers...) } @@ -709,12 +706,17 @@ func deduplicatedAfterCollection(supportBundle *troubleshootv1beta2.SupportBundl return b } -func getDefaultAnalyzers(isKurl bool) []*troubleshootv1beta2.Analyze { - defaultAnalyzers := defaultspec.Get().Spec.Analyzers +func getDefaultAnalyzers(isKurl, isAirgap bool) ([]*troubleshootv1beta2.Analyze, error) { + defaultSpec, err := defaultspec.Get(isAirgap) + if err != nil { + return nil, err + } + + defaultAnalyzers := defaultSpec.Spec.Analyzers if !isKurl { defaultAnalyzers = removeKurlAnalyzers(defaultAnalyzers) } - return defaultAnalyzers + return defaultAnalyzers, nil } // addDefaultDynamicTroubleshoot adds dynamic spec to the support bundle. diff --git a/pkg/supportbundle/staticspecs/embed.go b/pkg/supportbundle/staticspecs/embed.go index 778e28b70d..d0e1d74da5 100644 --- a/pkg/supportbundle/staticspecs/embed.go +++ b/pkg/supportbundle/staticspecs/embed.go @@ -24,8 +24,8 @@ var vendorspec []byte //go:embed defaultspec.yaml var defaultspec []byte -func GetVendorSpec() (*troubleshootv1beta2.SupportBundle, error) { - return supportbundle.ParseSupportBundleFromDoc(vendorspec) +func GetVendorSpec(isAirgap bool) (*troubleshootv1beta2.SupportBundle, error) { + return supportbundle.ParseSupportBundle(vendorspec, !isAirgap) } func GetClusterSpecificSpec(app *apptypes.App) (*troubleshootv1beta2.SupportBundle, error) { diff --git a/pkg/supportbundle/supportbundle.go b/pkg/supportbundle/supportbundle.go index 188545eb1d..df3e990753 100644 --- a/pkg/supportbundle/supportbundle.go +++ b/pkg/supportbundle/supportbundle.go @@ -312,7 +312,12 @@ func CreateSupportBundleAnalysis(appID string, archivePath string, bundle *types if err != nil { logger.Errorf("Failed to check if cluster is kurl: %v", err) } - analyzer.Spec.Analyzers = append(analyzer.Spec.Analyzers, getDefaultAnalyzers(isKurl)...) + + defaultAnalyzers, err := getDefaultAnalyzers(isKurl, foundApp.IsAirgap) + if err != nil { + return err + } + analyzer.Spec.Analyzers = append(analyzer.Spec.Analyzers, defaultAnalyzers...) analyzer.Spec.Analyzers = append(analyzer.Spec.Analyzers, getDefaultDynamicAnalyzers(foundApp)...) s := k8sjson.NewYAMLSerializer(k8sjson.DefaultMetaFactory, scheme.Scheme, scheme.Scheme) From 85ac17c34410c2f2bf839aea79dcbacec3c92474 Mon Sep 17 00:00:00 2001 From: Ricardo Maraschini Date: Tue, 15 Oct 2024 11:03:11 +0200 Subject: [PATCH 2/2] chore: add wrapping to errors addressing pr comments. --- pkg/supportbundle/defaultspec/embed.go | 3 ++- pkg/supportbundle/spec.go | 2 +- pkg/supportbundle/supportbundle.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/supportbundle/defaultspec/embed.go b/pkg/supportbundle/defaultspec/embed.go index b41dff158a..3ac62f6c04 100644 --- a/pkg/supportbundle/defaultspec/embed.go +++ b/pkg/supportbundle/defaultspec/embed.go @@ -3,6 +3,7 @@ package defaultspec import ( _ "embed" + "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/supportbundle" ) @@ -17,7 +18,7 @@ func Get(isAirgap bool) (troubleshootv1beta2.SupportBundle, error) { var err error spec, err = supportbundle.ParseSupportBundle(raw, !isAirgap) if err != nil { - return troubleshootv1beta2.SupportBundle{}, err + return troubleshootv1beta2.SupportBundle{}, errors.Wrap(err, "failed to parse support bundle") } } return *spec.DeepCopy(), nil diff --git a/pkg/supportbundle/spec.go b/pkg/supportbundle/spec.go index cf127a6a34..e28d655152 100644 --- a/pkg/supportbundle/spec.go +++ b/pkg/supportbundle/spec.go @@ -709,7 +709,7 @@ func deduplicatedAfterCollection(supportBundle *troubleshootv1beta2.SupportBundl func getDefaultAnalyzers(isKurl, isAirgap bool) ([]*troubleshootv1beta2.Analyze, error) { defaultSpec, err := defaultspec.Get(isAirgap) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to get default spec") } defaultAnalyzers := defaultSpec.Spec.Analyzers diff --git a/pkg/supportbundle/supportbundle.go b/pkg/supportbundle/supportbundle.go index df3e990753..0584fe1278 100644 --- a/pkg/supportbundle/supportbundle.go +++ b/pkg/supportbundle/supportbundle.go @@ -315,7 +315,7 @@ func CreateSupportBundleAnalysis(appID string, archivePath string, bundle *types defaultAnalyzers, err := getDefaultAnalyzers(isKurl, foundApp.IsAirgap) if err != nil { - return err + return errors.Wrap(err, "failed to get default analyzers") } analyzer.Spec.Analyzers = append(analyzer.Spec.Analyzers, defaultAnalyzers...) analyzer.Spec.Analyzers = append(analyzer.Spec.Analyzers, getDefaultDynamicAnalyzers(foundApp)...)