Skip to content

Commit

Permalink
Handle special overrides for extraLabels and extraAnnotations
Browse files Browse the repository at this point in the history 
In normal cases, ClusterInstance input data from the ClusterRequest overrides
the defaults from the configmap. However, for the fields extraLabels and
extraAnnotations, the default values from the configmap should take precedence
if the same labels/annotations exist in both.

Add a function to handle these special overrides. If extraLabels or extraAnnotations
are present in both the ClusterRequest and the configmap, the values from
ClusterRequest will be overriden with the defaults.

The utility merge function is invoked after to perform the normal merging behavior,
where ClusterRequest overrides defaults if both exist.
  • Loading branch information
@Missxiaoguo
Missxiaoguo committed Sep 20, 2024
1 parent ac8a45e commit 83f2d01
Show file tree
Hide file tree
Showing 3 changed files with 366 additions and 1 deletion.
12 changes: 11 additions & 1 deletion internal/controllers/clusterrequest_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,24 @@ func (t *clusterRequestReconcilerTask) getMergedClusterInputData(
return nil, fmt.Errorf("failed to get template defaults from ConfigMap %s: %w", templateDefaultsCm, err)
}

if dataType == utils.ClusterInstanceDataType {
// Special handling for overrides of ClusterInstance's extraLabels and extraAnnotations.
// The clusterTemplateInputMap will be overridden with the values from defaut configmap
// if same labels/annotations exist in both.
if err := utils.OverrideClusterInstanceLabelsOrAnnotations(
clusterTemplateInputMap, clusterTemplateDefaultsMap); err != nil {
return nil, utils.NewInputError(err.Error())
}
}

// Get the merged cluster data
mergedClusterDataMap, err := mergeClusterTemplateInputWithDefaults(clusterTemplateInputMap, clusterTemplateDefaultsMap)
if err != nil {
return nil, utils.NewInputError("failed to merge data for %s: %s", dataType, err.Error())
}

t.logger.Info(
fmt.Sprintf("Merged the %s default data with the clusterTemplateInput data for ClusterRequest", dataType),
fmt.Sprintf("Merged the %s default data with the clusterTemplateInput data for ClusterRequest: %+v", dataType, mergedClusterDataMap),
slog.String("name", t.object.Name),
)
return mergedClusterDataMap, nil
Expand Down
64 changes: 64 additions & 0 deletions internal/controllers/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,70 @@ func DeepMergeSlices[K comparable, V any](dst, src []V, checkType bool) ([]V, er
return result, nil
}

// OverrideClusterInstanceLabelsOrAnnotations handles the overrides of ClusterInstance's extraLabels
// or extraAnnotations. It overrides the values in the ClusterRequest with those from the default
// configmap when the same labels/annotations exist in both. Labels/annotations that are not common
// between the default configmap and ClusterRequest are ignored.
func OverrideClusterInstanceLabelsOrAnnotations(dstClusterRequestInput, srcConfigmap map[string]any) error {
fields := []string{"extraLabels", "extraAnnotations"}

for _, field := range fields {
srcValue, existsSrc := srcConfigmap[field]
dstValue, existsDst := dstClusterRequestInput[field]
// Check only when both configmap and ClusterRequestInput contain the field
if existsSrc && existsDst {
dstMap, okDst := dstValue.(map[string]any)
srcMap, okSrc := srcValue.(map[string]any)
if !okDst || !okSrc {
return fmt.Errorf("type mismatch for field %s: (from ClusterRequest: %T, from default Configmap: %T)",
field, dstValue, srcValue)
}

// Iterate over the resource types (e.g., ManagedCluster, AgentClusterInstall)
// Check labels/annotations only if the resource exists in both
for resourceType, srcFields := range srcMap {
if dstFields, exists := dstMap[resourceType]; exists {
dstFieldsMap, okDstFields := dstFields.(map[string]any)
srcFieldsMap, okSrcFields := srcFields.(map[string]any)
if !okDstFields || !okSrcFields {
return fmt.Errorf("type mismatch for field %s: (from ClusterRequest: %T, from default Configmap: %T)",
field, dstValue, srcValue)
}

// Override ClusterRequestInput's values with defaults if the label/annotation key exists in both
for srcFieldKey, srcLabelValue := range srcFieldsMap {
if _, exists := dstFieldsMap[srcFieldKey]; exists {
dstFieldsMap[srcFieldKey] = srcLabelValue
}
}
}
}
}
}

// Process label/annotation overrides for nodes
dstNodes, dstExists := dstClusterRequestInput["nodes"]
srcNodes, srcExists := srcConfigmap["nodes"]
if dstExists && srcExists {
// Determine the minimum length to merge
minLen := len(dstNodes.([]any))
if len(srcNodes.([]any)) < minLen {
minLen = len(srcNodes.([]any))
}

for i := 0; i < minLen; i++ {
if err := OverrideClusterInstanceLabelsOrAnnotations(
dstNodes.([]any)[i].(map[string]any),
srcNodes.([]any)[i].(map[string]any),
); err != nil {
return fmt.Errorf("type mismatch for nodes: %w", err)
}
}
}

return nil
}

func CopyK8sSecret(ctx context.Context, c client.Client, secretName, sourceNamespace, targetNamespace string) error {
// Get the secret from the source namespace
secret := &corev1.Secret{}
Expand Down
291 changes: 291 additions & 0 deletions internal/controllers/utils/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1554,3 +1554,294 @@ var _ = Describe("GetLabelsForPolicies", func() {
})

})

var _ = Describe("OverrideClusterInstanceLabelsOrAnnotations", func() {
var (
dstClusterRequestInput map[string]any
srcConfigmap map[string]any
)

BeforeEach(func() {
dstClusterRequestInput = make(map[string]any)
srcConfigmap = make(map[string]any)
})

It("should override only existing keys", func() {
dstClusterRequestInput = map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1",
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation1": "value1",
},
},
"clusterName": "cluster-1",
}

srcConfigmap = map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "new_value1", // Existing key in dst
"label2": "value2", // New key, should be ignored
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation2": "value2", // New key, should be ignored
},
},
}

expected := map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "new_value1", // Overridden
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation1": "value1",
},
},
"clusterName": "cluster-1",
}

err := OverrideClusterInstanceLabelsOrAnnotations(dstClusterRequestInput, srcConfigmap)
Expect(err).ToNot(HaveOccurred())
Expect(dstClusterRequestInput).To(Equal(expected))
})

It("should not add new resource types to dstClusterRequestInput", func() {
dstClusterRequestInput = map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1",
},
},
"clusterName": "cluster-1",
}

srcConfigmap = map[string]any{
"extraLabels": map[string]any{
"AgentClusterInstall": map[string]any{
"label1": "value1", // New resource type, should be ignored
},
},
}

expected := map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1", // Should remain unchanged
},
},
"clusterName": "cluster-1",
}

err := OverrideClusterInstanceLabelsOrAnnotations(dstClusterRequestInput, srcConfigmap)
Expect(err).ToNot(HaveOccurred())
Expect(dstClusterRequestInput).To(Equal(expected))
})

It("should not add extraLabels/extraAnnotations field if not found in ClusterRequestInput", func() {
dstClusterRequestInput = map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1",
},
},
"clusterName": "cluster-1",
}

srcConfigmap = map[string]any{
"extraAnnotations": map[string]any{ // Field does not exist in dstClusterRequestInput
"ManagedCluster": map[string]any{
"annotation1": "value1",
},
},
}

expected := map[string]any{
"extraLabels": map[string]any{ // Should remain unchanged
"ManagedCluster": map[string]any{
"label1": "value1",
},
},
"clusterName": "cluster-1",
}

err := OverrideClusterInstanceLabelsOrAnnotations(dstClusterRequestInput, srcConfigmap)
Expect(err).ToNot(HaveOccurred())
Expect(dstClusterRequestInput).To(Equal(expected))
})

It("should merge nodes and handle nested labels/annotations", func() {
dstClusterRequestInput = map[string]any{
"clusterName": "cluster-1",
"nodes": []any{
map[string]any{
"hostName": "node1",
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1",
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation1": "value1",
},
},
},
map[string]any{
"hostName": "node2",
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label2": "value2",
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation2": "value2",
},
},
},
},
}

srcConfigmap = map[string]any{
"nodes": []any{
map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "new_value1", // Existing label, should be overridden
"label2": "value2", // New label, should be ignored
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation2": "value2", // New annotation, should be ignored
},
},
},
map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1", // New label, should be ignored
"label2": "new_value2", // Existing label, should be overridden
},
},
},
},
}

expected := map[string]any{
"clusterName": "cluster-1",
"nodes": []any{
map[string]any{
"hostName": "node1",
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "new_value1", // Overridden
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation1": "value1", // no change
},
},
},
map[string]any{
"hostName": "node2",
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label2": "new_value2", // Overridden
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation2": "value2",
},
},
},
},
}

err := OverrideClusterInstanceLabelsOrAnnotations(dstClusterRequestInput, srcConfigmap)
Expect(err).ToNot(HaveOccurred())
Expect(dstClusterRequestInput).To(Equal(expected))
})

It("should not add the new node to dstClusterRequestInput", func() {
dstClusterRequestInput = map[string]any{
"clusterName": "cluster-1",
"nodes": []any{
map[string]any{
"hostName": "node1",
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1",
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation1": "value1",
},
},
},
},
}

srcConfigmap = map[string]any{
"nodes": []any{
map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "new_value1", // Existing label, should be overridden
"label2": "value2", // New label, should be ignored
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation2": "value2", // New annotation, should be ignored
},
},
},
// New node, should be ignored
map[string]any{
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "value1",
"label2": "value2",
},
},
},
},
}

expected := map[string]any{
"clusterName": "cluster-1",
"nodes": []any{
map[string]any{
"hostName": "node1",
"extraLabels": map[string]any{
"ManagedCluster": map[string]any{
"label1": "new_value1", // Overridden
},
},
"extraAnnotations": map[string]any{
"ManagedCluster": map[string]any{
"annotation1": "value1", // no change
},
},
},
},
}

err := OverrideClusterInstanceLabelsOrAnnotations(dstClusterRequestInput, srcConfigmap)
Expect(err).ToNot(HaveOccurred())
Expect(dstClusterRequestInput).To(Equal(expected))
})
})

0 comments on commit 83f2d01

Please sign in to comment.