Skip to content

Commit

Permalink
bug: use helm chart namespace when collecting resources
Browse files Browse the repository at this point in the history
When a ClusterProfile is using configuration drift detection:

1. addon-controller gets list of deployed resources
2. passes this information in a ResourceSummary to drift-detection-manager
3. drift-detection-manager starts watching those resources and when it
detects a configuration drift, drift-detection-manager reports it to
management cluster causing a new reconciliation

With respect to Helm charts, addon-controller gets list of deployed
resources using helm SDK and using manifest.
In certain scenarios, like this [one](projectsveltos/addon-controller#363)
manifest does not contain namespace for namespace resources like deployments.

If namespace is not set for namespace resource, point projectsveltos#3 won't work.

This PR fixes that. When drift-detection-manager gets resources deployed
by addon-controller because of an Helm chart, it adds the helm chart namespace.
This information is later on processed with dynamic.ResourceInterface which
ignores namespace for cluster wide.
  • Loading branch information
mgianluc committed Oct 15, 2023
1 parent 02ff041 commit c090a5b
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 0 deletions.
1 change: 1 addition & 0 deletions controllers/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var (
CleanMaps = (*ResourceSummaryReconciler).cleanMaps
GetResources = (*ResourceSummaryReconciler).getResources
GetHelmResources = (*ResourceSummaryReconciler).getHelmResources
GetChartResource = (*ResourceSummaryReconciler).getChartResource

GetKeyFromObject = getKeyFromObject
)
16 changes: 16 additions & 0 deletions controllers/resourcesummary_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,22 @@ func (r *ResourceSummaryReconciler) getChartResource(helmResource *libsveltosv1a

copy(resources, helmResource.Resources)

// Helm Resources are taken by addon-controller from helm manifest
// and passed to drift-detection-manager. There are cases where
// namespace is not set even for namespace resources. See this
// for instance: https://github.com/projectsveltos/addon-controller/issues/363
// Instead of changing addon-controller which would require querying the
// api-server to understand if a resource is namespace or cluster wide,
// we add namespace here.
// From here on, returned slice will only be used in conjunction with
// dynamic.ResourceInterface which ignores namespace for cluster wide
// resources
for i := range resources {
if resources[i].Namespace == "" {
resources[i].Namespace = helmResource.ReleaseNamespace
}
}

return resources
}

Expand Down
50 changes: 50 additions & 0 deletions controllers/resourcesummary_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,54 @@ var _ = Describe("ResourceSummary Reconciler", func() {
),
).Should(BeTrue())
})

It("getChartResource collects resources", func() {
c := fake.NewClientBuilder().WithScheme(scheme).Build()

reconciler := &controllers.ResourceSummaryReconciler{
Client: c,
Scheme: scheme,
Mux: sync.RWMutex{},
ResourceSummaryMap: make(map[corev1.ObjectReference]*libsveltosset.Set),
HelmResourceSummaryMap: make(map[corev1.ObjectReference]*libsveltosset.Set),
}

resource1 := libsveltosv1alpha1.Resource{
Name: randomString(),
Namespace: randomString(),
Group: randomString(),
Kind: randomString(),
Version: randomString(),
}

resource2 := libsveltosv1alpha1.Resource{
Name: randomString(),
Group: randomString(),
Kind: randomString(),
Version: randomString(),
}

helmResources := &libsveltosv1alpha1.HelmResources{
ChartName: randomString(),
ReleaseName: randomString(),
ReleaseNamespace: randomString(),
Resources: []libsveltosv1alpha1.Resource{
resource1, resource2,
},
}
resources := controllers.GetChartResource(reconciler, helmResources)

// When namespace is set, Resource is taken as it is
Expect(resources).To(ContainElement(resource1))

// When namespace is not set, helm chart namespace is used
tmpResource2 := libsveltosv1alpha1.Resource{
Name: resource2.Name,
Namespace: helmResources.ReleaseNamespace,
Group: resource2.Group,
Kind: resource2.Kind,
Version: resource2.Version,
}
Expect(resources).To(ContainElement(tmpResource2))
})
})

0 comments on commit c090a5b

Please sign in to comment.