Skip to content

Commit

Permalink
Don't surface errors that are not relevant to application
Browse files Browse the repository at this point in the history
  • Loading branch information
@divolgin
divolgin committed Dec 18, 2021
1 parent ca73b5d commit 4838706
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 33 deletions.
75 changes: 53 additions & 22 deletions pkg/supportbundle/spec.go
View file
Expand Up @@ -66,20 +66,23 @@ func CreateRenderedSpec(appID string, sequence int64, kotsKinds *kotsutil.KotsKi
return nil, errors.Wrap(err, "failed to get k8s clientset")
}

minimalRBACNamespaces := []string{}
if !k8sutil.IsKotsadmClusterScoped(context.TODO(), clientset, util.PodNamespace) {
minimalRBACNamespaces = append(minimalRBACNamespaces, util.PodNamespace)
minimalRBACNamespaces = append(minimalRBACNamespaces, kotsKinds.KotsApplication.Spec.AdditionalNamespaces...)
requiresAccess, err := kotssnapshot.CheckKotsadmVeleroAccess(context.TODO(), util.PodNamespace)
if err != nil {
logger.Errorf("Failed to check kotsadm velero access for the support bundle: %v", err)
} else if !requiresAccess {
namespacesToCollect := []string{}
namespacesToAnalyze := []string{}
if !kotsutil.IsKurl(clientset) {
// with cluster access, collect everything, but only analyze application namespaces
// with minimal RBAC collect only application namespaces
if k8sutil.IsKotsadmClusterScoped(context.TODO(), clientset, util.PodNamespace) {
namespacesToAnalyze = append(namespacesToAnalyze, util.PodNamespace)
namespacesToAnalyze = append(namespacesToAnalyze, kotsKinds.KotsApplication.Spec.AdditionalNamespaces...)
veleroNamespace, err := kotssnapshot.DetectVeleroNamespace(context.TODO(), clientset, util.PodNamespace)
if err != nil {
logger.Errorf("Failed to detect velero namespace for the support bundle: %v", err)
} else {
minimalRBACNamespaces = append(minimalRBACNamespaces, veleroNamespace)
namespacesToAnalyze = append(namespacesToAnalyze, veleroNamespace)
}
} else {
namespacesToCollect = append(namespacesToCollect, util.PodNamespace)
namespacesToCollect = append(namespacesToCollect, kotsKinds.KotsApplication.Spec.AdditionalNamespaces...)
}
}

Expand All @@ -88,7 +91,7 @@ func CreateRenderedSpec(appID string, sequence int64, kotsKinds *kotsutil.KotsKi
return nil, errors.Wrap(err, "failed to get app")
}

builtBundle, err = injectDefaults(app, builtBundle, opts, minimalRBACNamespaces)
builtBundle, err = injectDefaults(app, builtBundle, opts, namespacesToCollect, namespacesToAnalyze)
if err != nil {
return nil, errors.Wrap(err, "failed to inject defaults")
}
Expand Down Expand Up @@ -172,7 +175,7 @@ func CreateRenderedSpec(appID string, sequence int64, kotsKinds *kotsutil.KotsKi
}

// injectDefaults injects the kotsadm default collectors/analyzers in the the support bundle specification.
func injectDefaults(app *apptypes.App, b *troubleshootv1beta2.SupportBundle, opts types.TroubleshootOptions, minimalRBACNamespaces []string) (*troubleshootv1beta2.SupportBundle, error) {
func injectDefaults(app *apptypes.App, b *troubleshootv1beta2.SupportBundle, opts types.TroubleshootOptions, namespacesToCollect []string, namespacesToAnalyze []string) (*troubleshootv1beta2.SupportBundle, error) {
supportBundle := b.DeepCopy()

clientset, err := k8sutil.GetClientset()
Expand Down Expand Up @@ -201,7 +204,7 @@ func injectDefaults(app *apptypes.App, b *troubleshootv1beta2.SupportBundle, opt

supportBundle = addDefaultTroubleshoot(supportBundle, imageName, pullSecret)
supportBundle = addDefaultDynamicTroubleshoot(supportBundle, app, imageName, pullSecret)
supportBundle = populateNamespaces(supportBundle, minimalRBACNamespaces)
supportBundle = populateNamespaces(supportBundle, namespacesToCollect, namespacesToAnalyze)
supportBundle = deduplicatedCollectors(supportBundle)
supportBundle = deduplicatedAnalyzers(supportBundle)

Expand Down Expand Up @@ -238,7 +241,7 @@ func injectDefaults(app *apptypes.App, b *troubleshootv1beta2.SupportBundle, opt

// if a namespace is not set for a secret/run/logs/exec/copy collector, set it to the current namespace
// if kotsadm is running with minimal rbac priviliges, only collect resources from the specified minimal rbac namespaces
func populateNamespaces(supportBundle *troubleshootv1beta2.SupportBundle, minimalRBACNamespaces []string) *troubleshootv1beta2.SupportBundle {
func populateNamespaces(supportBundle *troubleshootv1beta2.SupportBundle, namespacesToCollect []string, namespacesToAnalyze []string) *troubleshootv1beta2.SupportBundle {
next := supportBundle.DeepCopy()

// collectors
Expand All @@ -262,26 +265,54 @@ func populateNamespaces(supportBundle *troubleshootv1beta2.SupportBundle, minima
if collect.Copy != nil && collect.Copy.Namespace == "" {
collect.Copy.Namespace = util.PodNamespace
}
if len(minimalRBACNamespaces) > 0 {
if len(namespacesToCollect) > 0 {
if collect.ClusterResources != nil && len(collect.ClusterResources.Namespaces) == 0 {
collect.ClusterResources.Namespaces = minimalRBACNamespaces
collect.ClusterResources.Namespaces = namespacesToCollect
}
}
collects = append(collects, collect)
}
next.Spec.Collectors = collects

// analyzers
var analyzers []*troubleshootv1beta2.Analyze
for _, analyzer := range next.Spec.Analyzers {
if len(minimalRBACNamespaces) > 0 {
if analyzer.ClusterPodStatuses != nil && len(analyzer.ClusterPodStatuses.Namespaces) == 0 {
analyzer.ClusterPodStatuses.Namespaces = minimalRBACNamespaces
if len(namespacesToAnalyze) > 0 {
isEmpty := func(namespace string, namespaces []string) bool {
if len(namespace) > 0 {
return false
}
if len(namespaces) > 0 {
return false
}
return true
}
analyzers = append(analyzers, analyzer)

var analyzers []*troubleshootv1beta2.Analyze
for _, analyzer := range next.Spec.Analyzers {

if analyzer.ClusterPodStatuses != nil && isEmpty("", analyzer.ClusterPodStatuses.Namespaces) {
analyzer.ClusterPodStatuses.Namespaces = namespacesToAnalyze
}

if analyzer.DeploymentStatus != nil && isEmpty(analyzer.DeploymentStatus.Namespace, analyzer.DeploymentStatus.Namespaces) {
analyzer.DeploymentStatus.Namespaces = namespacesToAnalyze
}

if analyzer.JobStatus != nil && isEmpty(analyzer.JobStatus.Namespace, analyzer.JobStatus.Namespaces) {
analyzer.JobStatus.Namespaces = namespacesToAnalyze
}

if analyzer.ReplicaSetStatus != nil && isEmpty(analyzer.ReplicaSetStatus.Namespace, analyzer.ReplicaSetStatus.Namespaces) {
analyzer.ReplicaSetStatus.Namespaces = namespacesToAnalyze
}

if analyzer.StatefulsetStatus != nil && isEmpty(analyzer.StatefulsetStatus.Namespace, analyzer.StatefulsetStatus.Namespaces) {
analyzer.StatefulsetStatus.Namespaces = namespacesToAnalyze
}

analyzers = append(analyzers, analyzer)
}
next.Spec.Analyzers = analyzers
}
next.Spec.Analyzers = analyzers

return next
}
Expand Down
102 changes: 91 additions & 11 deletions pkg/supportbundle/spec_test.go
View file
Expand Up @@ -17,14 +17,15 @@ func TestBuilder_populateNamespaces(t *testing.T) {
}()

tests := []struct {
name string
minimalRBACNamespaces []string
supportBundle *troubleshootv1beta2.SupportBundle
want *troubleshootv1beta2.SupportBundle
name string
namespacesToCollect []string
namespacesToAnalyze []string
supportBundle *troubleshootv1beta2.SupportBundle
want *troubleshootv1beta2.SupportBundle
}{
{
name: "all",
minimalRBACNamespaces: []string{},
name: "all",
namespacesToCollect: []string{},
supportBundle: &troubleshootv1beta2.SupportBundle{
Spec: troubleshootv1beta2.SupportBundleSpec{
Collectors: []*troubleshootv1beta2.Collect{
Expand Down Expand Up @@ -75,8 +76,8 @@ func TestBuilder_populateNamespaces(t *testing.T) {
},
},
{
name: "minimal rbac namespaces - preserve",
minimalRBACNamespaces: []string{"rbac-namespace-1", "rbac-namespace-2"},
name: "minimal rbac namespaces - preserve",
namespacesToCollect: []string{"rbac-namespace-1", "rbac-namespace-2"},
supportBundle: &troubleshootv1beta2.SupportBundle{
Spec: troubleshootv1beta2.SupportBundleSpec{
Collectors: []*troubleshootv1beta2.Collect{
Expand All @@ -101,25 +102,104 @@ func TestBuilder_populateNamespaces(t *testing.T) {
},
},
{
name: "minimal rbac namespaces - override",
minimalRBACNamespaces: []string{"rbac-namespace-1", "rbac-namespace-2"},
name: "minimal rbac namespaces - override",
namespacesToCollect: []string{"rbac-namespace-1", "rbac-namespace-2", "rbac-namespace-3"},
namespacesToAnalyze: []string{"rbac-namespace-1", "rbac-namespace-2"},
supportBundle: &troubleshootv1beta2.SupportBundle{
Spec: troubleshootv1beta2.SupportBundleSpec{
Collectors: []*troubleshootv1beta2.Collect{
{
ClusterResources: &troubleshootv1beta2.ClusterResources{},
},
},
Analyzers: []*troubleshootv1beta2.Analyze{
// these will be assigned namespaces
{
DeploymentStatus: &troubleshootv1beta2.DeploymentStatus{},
},
{
JobStatus: &troubleshootv1beta2.JobStatus{},
},
{
ReplicaSetStatus: &troubleshootv1beta2.ReplicaSetStatus{},
},
{
StatefulsetStatus: &troubleshootv1beta2.StatefulsetStatus{},
},
// these will not be assigned namespaces
{
DeploymentStatus: &troubleshootv1beta2.DeploymentStatus{
Namespaces: []string{"different-namespace-1", "different-namespace-2"},
},
},
{
JobStatus: &troubleshootv1beta2.JobStatus{
Namespace: "different-namespace-1",
},
},
{
ReplicaSetStatus: &troubleshootv1beta2.ReplicaSetStatus{
Namespaces: []string{"different-namespace-1", "different-namespace-2"},
},
},
{
StatefulsetStatus: &troubleshootv1beta2.StatefulsetStatus{
Namespace: "different-namespace-1",
},
},
},
},
},
want: &troubleshootv1beta2.SupportBundle{
Spec: troubleshootv1beta2.SupportBundleSpec{
Collectors: []*troubleshootv1beta2.Collect{
{
ClusterResources: &troubleshootv1beta2.ClusterResources{
Namespaces: []string{"rbac-namespace-1", "rbac-namespace-2", "rbac-namespace-3"},
},
},
},
Analyzers: []*troubleshootv1beta2.Analyze{
{
DeploymentStatus: &troubleshootv1beta2.DeploymentStatus{
Namespaces: []string{"rbac-namespace-1", "rbac-namespace-2"},
},
},
{
JobStatus: &troubleshootv1beta2.JobStatus{
Namespaces: []string{"rbac-namespace-1", "rbac-namespace-2"},
},
},
{
ReplicaSetStatus: &troubleshootv1beta2.ReplicaSetStatus{
Namespaces: []string{"rbac-namespace-1", "rbac-namespace-2"},
},
},
{
StatefulsetStatus: &troubleshootv1beta2.StatefulsetStatus{
Namespaces: []string{"rbac-namespace-1", "rbac-namespace-2"},
},
},
{
DeploymentStatus: &troubleshootv1beta2.DeploymentStatus{
Namespaces: []string{"different-namespace-1", "different-namespace-2"},
},
},
{
JobStatus: &troubleshootv1beta2.JobStatus{
Namespace: "different-namespace-1",
},
},
{
ReplicaSetStatus: &troubleshootv1beta2.ReplicaSetStatus{
Namespaces: []string{"different-namespace-1", "different-namespace-2"},
},
},
{
StatefulsetStatus: &troubleshootv1beta2.StatefulsetStatus{
Namespace: "different-namespace-1",
},
},
},
},
},
Expand All @@ -130,7 +210,7 @@ func TestBuilder_populateNamespaces(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
req := require.New(t)

got := populateNamespaces(tt.supportBundle, tt.minimalRBACNamespaces)
got := populateNamespaces(tt.supportBundle, tt.namespacesToCollect, tt.namespacesToAnalyze)

req.Equal(tt.want, got)
})
Expand Down

0 comments on commit 4838706

Please sign in to comment.