Apply upstream GitOpsSets bug fixes

api/v1alpha1/gitopsset_types.go

@@ -144,6 +144,12 @@ type APIClientGenerator struct {
 	// Reference to Secret in same namespace with a field "caFile" which
 	// provides the Certificate Authority to trust when making API calls.
 	SecretRef *LocalObjectReference `json:"secretRef,omitempty"`
+
+	// AllowClusterNetwork permits requests to loopback, private, and in-cluster
+	// addresses when true. If omitted or false, those destinations are denied.
+	// Link-local, unspecified, and multicast addresses are always denied.
+	// +optional
+	AllowClusterNetwork *bool `json:"allowClusterNetwork,omitempty"`
 }
 
 // HeadersReference references either a Secret or ConfigMap to be used for

config/crd/bases/sets.gitops.pro_gitopssets.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.19.0
+    controller-gen.kubebuilder.io/version: v0.22.0
   name: gitopssets.sets.gitops.pro
 spec:
   group: sets.gitops.pro
@@ -63,6 +63,12 @@ spec:
                         APIClientGenerator defines a generator that queries an API endpoint and uses
                         that to generate data.
                       properties:
+                        allowClusterNetwork:
+                          description: |-
+                            AllowClusterNetwork permits requests to loopback, private, and in-cluster
+                            addresses when true. If omitted or false, those destinations are denied.
+                            Link-local, unspecified, and multicast addresses are always denied.
+                          type: boolean
                         body:
                           description: |-
                             Body is set as the body in a POST request.
@@ -284,6 +290,12 @@ spec:
                                   APIClientGenerator defines a generator that queries an API endpoint and uses
                                   that to generate data.
                                 properties:
+                                  allowClusterNetwork:
+                                    description: |-
+                                      AllowClusterNetwork permits requests to loopback, private, and in-cluster
+                                      addresses when true. If omitted or false, those destinations are denied.
+                                      Link-local, unspecified, and multicast addresses are always denied.
+                                    type: boolean
                                   body:
                                     description: |-
                                       Body is set as the body in a POST request.

controllers/gitopsset_controller.go

@@ -124,12 +124,6 @@ func (r *GitOpsSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{Requeue: true}, nil
 	}
 
-	// Skip reconciliation if the GitOpsSet is suspended.
-	if gitOpsSet.Spec.Suspend {
-		logger.Info("Reconciliation is suspended for this GitOpsSet")
-		return ctrl.Result{}, nil
-	}
-
 	k8sClient := r.Client
 	if gitOpsSet.Spec.ServiceAccountName != "" || r.DefaultServiceAccount != "" {
 		serviceAccountName := r.DefaultServiceAccount
@@ -147,6 +141,12 @@ func (r *GitOpsSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return r.finalize(ctx, &gitOpsSet, k8sClient)
 	}
 
+	// Skip reconciliation if the GitOpsSet is suspended.
+	if gitOpsSet.Spec.Suspend {
+		logger.Info("Reconciliation is suspended for this GitOpsSet")
+		return ctrl.Result{}, nil
+	}
+
 	// Set the value of the reconciliation request in status.
 	if v, ok := fluxMeta.ReconcileAnnotationValue(gitOpsSet.GetAnnotations()); ok {
 		gitOpsSet.Status.LastHandledReconcileAt = v
@@ -370,7 +370,7 @@ func (r *GitOpsSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			handler.EnqueueRequestsFromMapFunc(r.secretToGitOpsSet),
 		)
 
-	if r.Generators["Config"] != nil {
+	if r.Generators["Config"] != nil || r.Generators["APIClient"] != nil {
 		builder.Watches(
 			&corev1.ConfigMap{},
 			handler.EnqueueRequestsFromMapFunc(r.configMapToGitOpsSet),
@@ -438,8 +438,7 @@ func (r *GitOpsSetReconciler) finalize(ctx context.Context, gs *templatesv1.GitO
 	logger := ctrl.LoggerFrom(ctx)
 	logger.Info("finalizing resources")
 
-	if !gs.Spec.Suspend &&
-		gs.Status.Inventory != nil &&
+	if gs.Status.Inventory != nil &&
 		gs.Status.Inventory.Entries != nil {
 
 		if err := r.removeResourceRefs(ctx, k8sClient, gs.Status.Inventory.Entries); err != nil {
@@ -623,27 +622,40 @@ func indexConfig(kind string) func(o client.Object) []string {
 			panic(fmt.Sprintf("Expected a GitOpsSet, got %T", o))
 		}
 
-		referencedResources := []*templatesv1.ConfigGenerator{}
-		for _, gen := range ks.Spec.Generators {
-			if gen.Config != nil && gen.Config.Kind == kind {
-				referencedResources = append(referencedResources, gen.Config)
+		referencedNames := []string{}
+		addConfig := func(config *templatesv1.ConfigGenerator) {
+			if config != nil && config.Kind == kind {
+				referencedNames = append(referencedNames, fmt.Sprintf("%s/%s", ks.GetNamespace(), config.Name))
 			}
-			if gen.Matrix != nil && gen.Matrix.Generators != nil {
-				for _, matrixGen := range gen.Matrix.Generators {
-					if matrixGen.Config != nil && matrixGen.Config.Kind == kind {
-						referencedResources = append(referencedResources, matrixGen.Config)
-					}
-				}
+		}
+		addSecret := func(secret *templatesv1.LocalObjectReference) {
+			if kind == "Secret" && secret != nil {
+				referencedNames = append(referencedNames, fmt.Sprintf("%s/%s", ks.GetNamespace(), secret.Name))
 			}
 		}
-
-		if len(referencedResources) == 0 {
-			return nil
+		addHeaders := func(headers *templatesv1.HeadersReference) {
+			if headers != nil && headers.Kind == kind {
+				referencedNames = append(referencedNames, fmt.Sprintf("%s/%s", ks.GetNamespace(), headers.Name))
+			}
+		}
+		addGeneratorReferences := func(pullRequests *templatesv1.PullRequestGenerator, apiClient *templatesv1.APIClientGenerator, config *templatesv1.ConfigGenerator) {
+			addConfig(config)
+			if pullRequests != nil {
+				addSecret(pullRequests.SecretRef)
+			}
+			if apiClient != nil {
+				addSecret(apiClient.SecretRef)
+				addHeaders(apiClient.HeadersRef)
+			}
 		}
 
-		referencedNames := []string{}
-		for _, grg := range referencedResources {
-			referencedNames = append(referencedNames, fmt.Sprintf("%s/%s", ks.GetNamespace(), grg.Name))
+		for _, gen := range ks.Spec.Generators {
+			addGeneratorReferences(gen.PullRequests, gen.APIClient, gen.Config)
+			if gen.Matrix != nil && gen.Matrix.Generators != nil {
+				for _, matrixGen := range gen.Matrix.Generators {
+					addGeneratorReferences(matrixGen.PullRequests, matrixGen.APIClient, matrixGen.Config)
+				}
+			}
 		}
 
 		return referencedNames

controllers/gitopsset_controller_test.go

@@ -1,6 +1,7 @@
 package controllers
 
 import (
+	"context"
 	"encoding/json"
 	"path/filepath"
 	"sort"
@@ -735,6 +736,35 @@ func TestReconciliation(t *testing.T) {
 		assertNoKustomizationsExistInNamespace(t, k8sClient, "default")
 	})
 
+	t.Run("deleting a suspended set cleans its inventory", func(t *testing.T) {
+		ctx := t.Context()
+		gs := createAndReconcileToFinalizedState(t, k8sClient, reconciler, makeTestGitOpsSet(t))
+		deleted := false
+		defer func() {
+			if !deleted {
+				deleteGitOpsSetAndFinalize(t, k8sClient, reconciler, gs)
+			}
+		}()
+
+		_, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gs)})
+		test.AssertNoError(t, err)
+		test.AssertNoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(gs), gs))
+		assertKustomizationsExist(t, k8sClient, "default", "engineering-dev-demo", "engineering-prod-demo", "engineering-preprod-demo")
+
+		gs.Spec.Suspend = true
+		test.AssertNoError(t, k8sClient.Update(ctx, gs))
+		test.AssertNoError(t, k8sClient.Delete(ctx, gs))
+
+		_, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gs)})
+		test.AssertNoError(t, err)
+
+		if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gs), &templatesv1.GitOpsSet{}); !apierrors.IsNotFound(err) {
+			t.Fatalf("GitOpsSet still exists after finalization: %v", err)
+		}
+		deleted = true
+		assertNoKustomizationsExistInNamespace(t, k8sClient, "default")
+	})
+
 	t.Run("reconciling when gitrepository has no artifact", func(t *testing.T) {
 		ctx := t.Context()
 		emptyGR := test.NewGitRepository()
@@ -887,6 +917,47 @@ func TestGetClusterSelectors(t *testing.T) {
 	}
 }
 
+func TestIndexConfigIncludesGeneratorReferences(t *testing.T) {
+	gitOpsSet := &templatesv1.GitOpsSet{
+		ObjectMeta: metav1.ObjectMeta{Namespace: "demo"},
+		Spec: templatesv1.GitOpsSetSpec{
+			Generators: []templatesv1.GitOpsSetGenerator{
+				{
+					PullRequests: &templatesv1.PullRequestGenerator{
+						SecretRef: &templatesv1.LocalObjectReference{Name: "pr-token"},
+					},
+					APIClient: &templatesv1.APIClientGenerator{
+						SecretRef:  &templatesv1.LocalObjectReference{Name: "api-ca"},
+						HeadersRef: &templatesv1.HeadersReference{Kind: "Secret", Name: "api-auth"},
+					},
+				},
+				{
+					Matrix: &templatesv1.MatrixGenerator{
+						Generators: []templatesv1.GitOpsSetNestedGenerator{
+							{
+								PullRequests: &templatesv1.PullRequestGenerator{
+									SecretRef: &templatesv1.LocalObjectReference{Name: "matrix-pr-token"},
+								},
+								APIClient: &templatesv1.APIClientGenerator{
+									SecretRef:  &templatesv1.LocalObjectReference{Name: "matrix-api-ca"},
+									HeadersRef: &templatesv1.HeadersReference{Kind: "ConfigMap", Name: "matrix-api-headers"},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	if got, want := indexConfig("Secret")(gitOpsSet), []string{"demo/pr-token", "demo/api-ca", "demo/api-auth", "demo/matrix-pr-token", "demo/matrix-api-ca"}; cmp.Diff(want, got) != "" {
+		t.Fatalf("Secret index mismatch (-want +got):\n%s", cmp.Diff(want, got))
+	}
+	if got, want := indexConfig("ConfigMap")(gitOpsSet), []string{"demo/matrix-api-headers"}; cmp.Diff(want, got) != "" {
+		t.Fatalf("ConfigMap index mismatch (-want +got):\n%s", cmp.Diff(want, got))
+	}
+}
+
 func TestMatchCluster(t *testing.T) {
 	gitopsCluster := &clustersv1.GitopsCluster{
 		ObjectMeta: metav1.ObjectMeta{
@@ -1135,7 +1206,9 @@ func assertInventoryHasNoItems(t *testing.T, gs *templatesv1.GitOpsSet) {
 
 func deleteObject(t *testing.T, cl client.Client, obj client.Object) {
 	t.Helper()
-	if err := cl.Delete(t.Context(), obj); err != nil {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	if err := cl.Delete(ctx, obj); err != nil {
 		t.Fatal(err)
 	}
 }

controllers/templates/renderer.go

@@ -161,6 +161,9 @@ func renderTemplateParams(index int, tmpl templatesv1.GitOpsSetTemplate, params
 				return nil, fmt.Errorf("failed convert parsed template: %w", err)
 			}
 			delete(unstructuredMap, "status")
+			if metadata, ok := unstructuredMap["metadata"].(map[string]any); ok && metadata["creationTimestamp"] == nil {
+				delete(metadata, "creationTimestamp")
+			}
 			uns := &unstructured.Unstructured{Object: unstructuredMap}
 
 			if IsNamespacedObject(uns) {
@@ -240,6 +243,9 @@ func makeTemplateFunctions() template.FuncMap {
 	f := sprig.TxtFuncMap()
 	unwanted := []string{
 		"env", "expandenv", "getHostByName", "genPrivateKey", "derivePassword", "sha256sum",
+		"genCA", "genSelfSignedCert", "genSignedCert", "genCAWithKey", "genSelfSignedCertWithKey", "genSignedCertWithKey",
+		"bcrypt", "htpasswd", "uuidv4", "now", "date", "dateInZone", "dateModify", "ago", "unixEpoch",
+		"randInt", "randBytes", "randNumeric", "randAlpha", "randAlphaNum", "randAscii",
 		"base", "dir", "ext", "clean", "isAbs", "osBase", "osDir", "osExt", "osClean", "osIsAbs"}
 
 	for _, v := range unwanted {
@@ -254,13 +260,12 @@ func makeTemplateFunctions() template.FuncMap {
 
 		return def
 	}
-	f["toYaml"] = func(v interface{}) string {
+	f["toYaml"] = func(v interface{}) (string, error) {
 		data, err := syaml.Marshal(v)
 		if err != nil {
-			// Swallow errors inside of a template.
-			return ""
+			return "", err
 		}
-		return strings.TrimSuffix(string(data), "\n")
+		return strings.TrimSuffix(string(data), "\n"), nil
 	}
 
 	return f

controllers/templates/renderer_test.go

@@ -29,6 +29,41 @@ const (
 	testNS            = "demo"
 )
 
+func TestTemplateFunctionsAreStable(t *testing.T) {
+	for _, name := range []string{
+		"genCA", "genSelfSignedCert", "genSignedCert", "genCAWithKey", "genSelfSignedCertWithKey", "genSignedCertWithKey",
+		"bcrypt", "htpasswd", "uuidv4", "now", "date", "dateInZone", "dateModify", "ago", "unixEpoch",
+		"randInt", "randBytes", "randNumeric", "randAlpha", "randAlphaNum", "randAscii",
+	} {
+		if _, ok := makeTemplateFunctions()[name]; ok {
+			t.Errorf("template function %q should be unavailable", name)
+		}
+	}
+}
+
+func TestToYAMLReturnsMarshalErrors(t *testing.T) {
+	_, err := render([]byte(`value: {{ toYaml .Value }}`), map[string]any{"Value": make(chan int)}, templatesv1.GitOpsSet{})
+	if err == nil {
+		t.Fatal("render() succeeded for a value that cannot be marshaled as YAML")
+	}
+}
+
+func TestRenderTemplateParamsDropsNullCreationTimestamp(t *testing.T) {
+	objects, err := renderTemplateParams(0, templatesv1.GitOpsSetTemplate{
+		Content: runtime.RawExtension{Raw: []byte(`{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"example","creationTimestamp":null}}`)},
+	}, map[string]any{}, templatesv1.GitOpsSet{ObjectMeta: metav1.ObjectMeta{Name: "example-set", Namespace: "demo"}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	metadata, ok := objects[0].Object["metadata"].(map[string]any)
+	if !ok {
+		t.Fatalf("metadata has unexpected type: %T", objects[0].Object["metadata"])
+	}
+	if _, exists := metadata["creationTimestamp"]; exists {
+		t.Fatal("rendered metadata still contains creationTimestamp")
+	}
+}
+
 func TestRender(t *testing.T) {
 	testGenerators := map[string]generators.Generator{
 		"List": list.NewGenerator(logr.Discard()),

docs/api/gitopsset.md

@@ -266,6 +266,20 @@ LocalObjectReference
 provides the Certificate Authority to trust when making API calls.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>allowClusterNetwork</code><br />
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>AllowClusterNetwork permits requests to loopback, private, and in-cluster
+addresses when true. If omitted or false, those destinations are denied.
+Link-local, unspecified, and multicast addresses are always denied.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="sets.gitops.pro/v1alpha1.ClusterGenerator">ClusterGenerator