Skip to content

Commit

Permalink
(bug) when deploymentType is Local, validate namespace
Browse files Browse the repository at this point in the history
A Profile using deploymentType: Local, can only deploy resources
in the same namespace.

Previously, Sveltos used the following format for the OwnerReferences
name for resources it deployed due to a Profile:

```
Profile.namespace/Profile.name
```

However, a Profile with this exact name doesn't exist within the
management cluster. As a result, the Kubernetes controller would
immediately remove the deployed resource.

This behavior occurred because:

```
A namespaced owner must exist in the same namespace as the dependent.
If it does not, the owner reference is treated as absent, and the
dependent is subject to deletion once all owners are verified absent
```

Given this PR ensures that a Profile only creates resources in the management
cluster in the same namespace, now only Profile.name is set in the
OwnerReference.name
  • Loading branch information
mgianluc committed Nov 17, 2024
1 parent 1b35cf6 commit 70f72fc
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 29 deletions.
27 changes: 20 additions & 7 deletions controllers/handlers_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,21 @@ func adjustNamespace(policy *unstructured.Unstructured, destConfig *rest.Config)
return nil
}

// isResourceNamespaceValid validates the resource namespace.
// A Profile, when deploying resources locally, i.e, to the management cluster, can
// only deploy resources in the same namespace
func isResourceNamespaceValid(profile client.Object, policy *unstructured.Unstructured,
deployingToMgmtCluster bool) bool {

if profile.GetObjectKind().GroupVersionKind().Kind == configv1beta1.ProfileKind {
if deployingToMgmtCluster && policy.GetNamespace() != profile.GetNamespace() {
return false
}
}

return true
}

// deployUnstructured deploys referencedUnstructured objects.
// Returns an error if one occurred. Otherwise it returns a slice containing the name of
// the policies deployed in the form of kind.group:namespace:name for namespaced policies
Expand All @@ -373,9 +388,6 @@ func deployUnstructured(ctx context.Context, deployingToMgmtCluster bool, destCo
if err != nil {
return nil, err
}
if profile.GetObjectKind().GroupVersionKind().Kind == configv1beta1.ProfileKind {
profile.SetName(profileNameToOwnerReferenceName(profile))
}

patches, err := initiatePatches(ctx, clusterSummary, "patch", mgmtResources, logger)
if err != nil {
Expand All @@ -400,6 +412,10 @@ func deployUnstructured(ctx context.Context, deployingToMgmtCluster bool, destCo
return nil, err
}

if !isResourceNamespaceValid(profile, policy, deployingToMgmtCluster) {
return nil, fmt.Errorf("profile can only deploy resource in same namespace in the management cluster")
}

logger.V(logs.LogDebug).Info(fmt.Sprintf("deploying resource %s %s/%s (deploy to management cluster: %v)",
policy.GetKind(), policy.GetNamespace(), policy.GetName(), deployingToMgmtCluster))

Expand Down Expand Up @@ -517,7 +533,7 @@ func requeueAllOldOwners(ctx context.Context, profileOwners []corev1.ObjectRefer
profileName = types.NamespacedName{Name: profileOwners[i].Name}
case configv1beta1.ProfileKind:
profileKind = configv1beta1.ProfileKind
profileName = *getProfileNameFromOwnerReferenceName(profileOwners[i].Name)
profileName = types.NamespacedName{Namespace: clusterSummary.Namespace, Name: profileOwners[i].Name}
default:
continue
}
Expand Down Expand Up @@ -1066,9 +1082,6 @@ func undeployStaleResources(ctx context.Context, isMgmtCluster bool,
if err != nil {
return nil, err
}
if profile.GetObjectKind().GroupVersionKind().Kind == configv1beta1.ProfileKind {
profile.SetName(profileNameToOwnerReferenceName(profile))
}

undeployed := make([]configv1beta1.ResourceReport, 0)

Expand Down
22 changes: 0 additions & 22 deletions controllers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,28 +393,6 @@ func parseMapFromString(data string) (map[string]string, error) {
return result, nil
}

// Sveltos deployment in managed clusters relies on OwnerReferences to track the responsible profile.
// However, a limitation arises with namespaced Profiles.
// Kubernetes OwnerReferences lack a namespace field, assuming owners reside in the same namespace.
// For Profile resources (namespaced), Sveltos dynamically modifies the owner name to incorporate both
// namespace and name for proper identification.
func profileNameToOwnerReferenceName(profile client.Object) string {
if profile.GetObjectKind().GroupVersionKind().Kind == configv1beta1.ProfileKind {
return fmt.Sprintf("%s/%s", profile.GetNamespace(), profile.GetName())
}

return profile.GetName()
}

func getProfileNameFromOwnerReferenceName(profileName string) *types.NamespacedName {
result := strings.Split(profileName, "/")
if len(result) == 1 {
// resources deployed by Sveltos before release v0.30.0 did not have profile namespace/name
return &types.NamespacedName{Name: profileName}
}
return &types.NamespacedName{Namespace: result[0], Name: result[1]}
}

// Function to remove duplicates from a slice
func unique[T comparable](input []T) []T {
seen := make(map[T]bool)
Expand Down

0 comments on commit 70f72fc

Please sign in to comment.