feat: [sc-103119] URI is not replacing the spec but merging instead (#1541)

* only replace spec that has uri

* add unit test

* fix unit tests from code review
This commit is contained in:
Gerard Nguyen
2024-05-03 11:36:27 +01:00
committed by GitHub
parent aeaac7a70f
commit 2d05e38a09
2 changed files with 103 additions and 29 deletions
+31 -24
View File
@@ -245,35 +245,42 @@ the %s Admin Console to begin analysis.`
// loadSupportBundleSpecsFromURIs loads support bundle specs from URIs
func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.TroubleshootKinds) error {
remoteRawSpecs := []string{}
moreKinds := loader.NewTroubleshootKinds()
// iterate through original kinds and replace any support bundle spec with provided uri spec
for _, s := range kinds.SupportBundlesV1Beta2 {
if s.Spec.Uri != "" && util.IsURL(s.Spec.Uri) {
// We are using LoadSupportBundleSpec function here since it handles prompting
// users to accept insecure connections
// There is an opportunity to refactor this code in favour of the Loader APIs
// TODO: Pass ctx to LoadSupportBundleSpec
rawSpec, err := supportbundle.LoadSupportBundleSpec(s.Spec.Uri)
if err != nil {
// In the event a spec can't be loaded, we'll just skip it and print a warning
klog.Warningf("unable to load support bundle from URI: %q: %v", s.Spec.Uri, err)
continue
}
remoteRawSpecs = append(remoteRawSpecs, string(rawSpec))
if s.Spec.Uri == "" || !util.IsURL(s.Spec.Uri) {
moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, s)
continue
}
// We are using LoadSupportBundleSpec function here since it handles prompting
// users to accept insecure connections
// There is an opportunity to refactor this code in favour of the Loader APIs
// TODO: Pass ctx to LoadSupportBundleSpec
rawSpec, err := supportbundle.LoadSupportBundleSpec(s.Spec.Uri)
if err != nil {
// add back original spec
moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, s)
// In the event a spec can't be loaded, we'll just skip it and print a warning
klog.Warningf("unable to load support bundle from URI: %q: %v", s.Spec.Uri, err)
continue
}
k, err := loader.LoadSpecs(ctx, loader.LoadOptions{RawSpec: string(rawSpec)})
if err != nil {
// add back original spec
moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, s)
klog.Warningf("unable to load spec: %v", err)
continue
}
// finally append the uri spec
moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, k.SupportBundlesV1Beta2...)
}
if len(remoteRawSpecs) == 0 {
return nil
}
kinds.SupportBundlesV1Beta2 = moreKinds.SupportBundlesV1Beta2
moreKinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{
RawSpecs: remoteRawSpecs,
})
if err != nil {
return err
}
kinds.Add(moreKinds)
return nil
}
+72 -5
View File
@@ -18,7 +18,8 @@ import (
testclient "k8s.io/client-go/kubernetes/fake"
)
var orig = `
func templSpec() string {
return `
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
@@ -30,6 +31,7 @@ spec:
name: kube-root-ca.crt
namespace: default
`
}
func Test_loadSupportBundleSpecsFromURIs(t *testing.T) {
// Run a webserver to serve the spec
@@ -45,7 +47,7 @@ spec:
}))
defer srv.Close()
orig := strings.ReplaceAll(orig, "$MY_URI", srv.URL)
orig := strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL)
ctx := context.Background()
kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{RawSpec: orig})
@@ -57,8 +59,73 @@ spec:
err = loadSupportBundleSpecsFromURIs(ctx, kinds)
require.NoError(t, err)
require.Len(t, kinds.SupportBundlesV1Beta2, 2)
assert.NotNil(t, kinds.SupportBundlesV1Beta2[1].Spec.Collectors[0].ClusterInfo)
require.Len(t, kinds.SupportBundlesV1Beta2, 1)
assert.NotNil(t, kinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ClusterInfo)
}
func Test_loadMultipleSupportBundleSpecsWithNoURIs(t *testing.T) {
ctx := context.Background()
client := testclient.NewSimpleClientset()
specs := []string{testutils.ServeFromFilePath(t, `
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
name: sb-1
spec:
collectors:
- clusterInfo:{}`), testutils.ServeFromFilePath(t, `
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
name: sb-2
spec:
collectors:
- clusterInfo: {}`)}
sb, _, err := loadSpecs(ctx, specs, client)
require.NoError(t, err)
require.Len(t, sb.Spec.Collectors, 2)
}
func Test_loadMultipleSupportBundleSpecsWithURIs(t *testing.T) {
ctx := context.Background()
client := testclient.NewSimpleClientset()
specFile := testutils.ServeFromFilePath(t, `
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
name: sb-file
spec:
collectors:
- logs:
name: podlogs/kotsadm
selector:
- app=kotsadm
`)
// Run a webserver to serve the spec
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
name: sb-uri
spec:
collectors:
- clusterInfo: {}`))
}))
defer srv.Close()
orig := strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL)
specUri := testutils.ServeFromFilePath(t, orig)
specs := []string{specFile, specUri}
sb, _, err := loadSpecs(ctx, specs, client)
require.NoError(t, err)
assert.NotNil(t, sb.Spec.Collectors[0].Logs)
assert.Nil(t, sb.Spec.Collectors[1].ConfigMap) // original spec gone
assert.NotNil(t, sb.Spec.Collectors[1].ClusterInfo) // new spec from URI
}
func Test_loadSupportBundleSpecsFromURIs_TimeoutError(t *testing.T) {
@@ -69,7 +136,7 @@ func Test_loadSupportBundleSpecsFromURIs_TimeoutError(t *testing.T) {
ctx := context.Background()
kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{
RawSpec: strings.ReplaceAll(orig, "$MY_URI", srv.URL),
RawSpec: strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL),
})
require.NoError(t, err)