diff --git a/src/internal/agent/hooks/flux-helmrepo.go b/src/internal/agent/hooks/flux-helmrepo.go index 067ae0c890..f67a640384 100644 --- a/src/internal/agent/hooks/flux-helmrepo.go +++ b/src/internal/agent/hooks/flux-helmrepo.go @@ -37,6 +37,14 @@ func NewHelmRepositoryMutationHook(ctx context.Context, cluster *cluster.Cluster // mutateHelmRepo mutates the repository url to point to the repository URL defined in the ZarfState. func mutateHelmRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) { l := logger.From(ctx) + var ( + patches []operations.PatchOperation + isPatched bool + + isCreate = r.Operation == v1.Create + isUpdate = r.Operation == v1.Update + ) + src := &flux.HelmRepository{} if err := json.Unmarshal(r.Object.Raw, &src); err != nil { return nil, fmt.Errorf(lang.ErrUnmarshal, err) @@ -48,13 +56,6 @@ func mutateHelmRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluste return &operations.Result{Allowed: true}, nil } - if src.Labels != nil && src.Labels["zarf-agent"] == "patched" { - return &operations.Result{ - Allowed: true, - PatchOps: nil, - }, nil - } - zarfState, err := cluster.LoadZarfState(ctx) if err != nil { return nil, err @@ -70,21 +71,36 @@ func mutateHelmRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluste "name", src.Name, "registry", registryAddress) - patchedSrc, err := transform.ImageTransformHost(registryAddress, src.Spec.URL) - if err != nil { - return nil, fmt.Errorf("unable to transform the HelmRepo URL: %w", err) + patchedURL := src.Spec.URL + + // Check if this is an update operation and the hostname is different from what we have in the zarfState + // NOTE: We mutate on updates IF AND ONLY IF the hostname in the request is different than the hostname in the zarfState + // NOTE: We are checking if the hostname is different before because we do not want to potentially mutate a URL that has already been mutated. + if isUpdate { + zarfStateAddress := helpers.OCIURLPrefix + registryAddress + isPatched, err = helpers.DoHostnamesMatch(zarfStateAddress, src.Spec.URL) + if err != nil { + return nil, fmt.Errorf(lang.AgentErrHostnameMatch, err) + } } - patchedRefInfo, err := transform.ParseImageRef(patchedSrc) - if err != nil { - return nil, fmt.Errorf("unable to parse the HelmRepo URL: %w", err) + // Mutate the helm repo URL if necessary + if isCreate || (isUpdate && !isPatched) { + patchedSrc, err := transform.ImageTransformHost(registryAddress, src.Spec.URL) + if err != nil { + return nil, fmt.Errorf("unable to transform the HelmRepo URL: %w", err) + } + + patchedRefInfo, err := transform.ParseImageRef(patchedSrc) + if err != nil { + return nil, fmt.Errorf("unable to parse the HelmRepo URL: %w", err) + } + patchedURL = helpers.OCIURLPrefix + patchedRefInfo.Name } - patchedURL := helpers.OCIURLPrefix + patchedRefInfo.Name l.Debug("mutating the Flux HelmRepository URL to the Zarf URL", "original", src.Spec.URL, "mutated", patchedURL) - patches := populateHelmRepoPatchOperations(patchedURL, zarfState.RegistryInfo.IsInternal()) - + patches = populateHelmRepoPatchOperations(patchedURL, zarfState.RegistryInfo.IsInternal()) patches = append(patches, getLabelPatch(src.Labels)) return &operations.Result{ diff --git a/src/internal/agent/hooks/flux-helmrepo_test.go b/src/internal/agent/hooks/flux-helmrepo_test.go index d56d7e29a5..b5c861331f 100644 --- a/src/internal/agent/hooks/flux-helmrepo_test.go +++ b/src/internal/agent/hooks/flux-helmrepo_test.go @@ -68,22 +68,36 @@ func TestFluxHelmMutationWebhook(t *testing.T) { code: http.StatusInternalServerError, }, { - name: "should not mutate when agent patched", - admissionReq: createFluxHelmRepoAdmissionRequest(t, v1.Update, &flux.HelmRepository{ + name: "should be mutated with no internal service registry", + admissionReq: createFluxHelmRepoAdmissionRequest(t, v1.Create, &flux.HelmRepository{ ObjectMeta: metav1.ObjectMeta{ - Name: "already-patched", - Labels: map[string]string{ - "zarf-agent": "patched", - }, + Name: "mutate-this", }, Spec: flux.HelmRepositorySpec{ + URL: "oci://ghcr.io/stefanprodan/charts", Type: "oci", }, }), + patch: []operations.PatchOperation{ + operations.ReplacePatchOperation( + "/spec/url", + "oci://127.0.0.1:31999/stefanprodan/charts", + ), + operations.AddPatchOperation( + "/spec/secretRef", + fluxmeta.LocalObjectReference{Name: config.ZarfImagePullSecretName}, + ), + operations.ReplacePatchOperation( + "/metadata/labels", + map[string]string{ + "zarf-agent": "patched", + }, + ), + }, code: http.StatusOK, }, { - name: "should be mutated with no internal service registry", + name: "should be mutated with internal service registry", admissionReq: createFluxHelmRepoAdmissionRequest(t, v1.Create, &flux.HelmRepository{ ObjectMeta: metav1.ObjectMeta{ Name: "mutate-this", @@ -93,6 +107,55 @@ func TestFluxHelmMutationWebhook(t *testing.T) { Type: "oci", }, }), + patch: []operations.PatchOperation{ + operations.ReplacePatchOperation( + "/spec/url", + "oci://10.11.12.13:5000/stefanprodan/charts", + ), + operations.AddPatchOperation( + "/spec/secretRef", + fluxmeta.LocalObjectReference{Name: config.ZarfImagePullSecretName}, + ), + operations.ReplacePatchOperation( + "/metadata/labels", + map[string]string{ + "zarf-agent": "patched", + }, + ), + }, + svc: &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "zarf-docker-registry", + Namespace: "zarf", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + Ports: []corev1.ServicePort{ + { + NodePort: int32(31999), + Port: 5000, + }, + }, + ClusterIP: "10.11.12.13", + }, + }, + code: http.StatusOK, + }, + { + name: "should not mutate URL if it has the same hostname as Zarf state", + admissionReq: createFluxHelmRepoAdmissionRequest(t, v1.Update, &flux.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-mutate-this", + }, + Spec: flux.HelmRepositorySpec{ + URL: "oci://127.0.0.1:31999/stefanprodan/charts", + Type: "oci", + }, + }), patch: []operations.PatchOperation{ operations.ReplacePatchOperation( "/spec/url", @@ -112,13 +175,13 @@ func TestFluxHelmMutationWebhook(t *testing.T) { code: http.StatusOK, }, { - name: "should be mutated with internal service registry", - admissionReq: createFluxHelmRepoAdmissionRequest(t, v1.Create, &flux.HelmRepository{ + name: "should not mutate URL if it has the same hostname as Zarf state internal repo", + admissionReq: createFluxHelmRepoAdmissionRequest(t, v1.Update, &flux.HelmRepository{ ObjectMeta: metav1.ObjectMeta{ - Name: "mutate-this", + Name: "no-mutate-this", }, Spec: flux.HelmRepositorySpec{ - URL: "oci://ghcr.io/stefanprodan/charts", + URL: "oci://10.11.12.13:5000/stefanprodan/charts", Type: "oci", }, }), diff --git a/src/internal/agent/hooks/flux-ocirepo.go b/src/internal/agent/hooks/flux-ocirepo.go index 39cd139aaf..3024705ddc 100644 --- a/src/internal/agent/hooks/flux-ocirepo.go +++ b/src/internal/agent/hooks/flux-ocirepo.go @@ -36,6 +36,14 @@ func NewOCIRepositoryMutationHook(ctx context.Context, cluster *cluster.Cluster) // mutateOCIRepo mutates the oci repository url to point to the repository URL defined in the ZarfState. func mutateOCIRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) { l := logger.From(ctx) + var ( + patches []operations.PatchOperation + isPatched bool + + isCreate = r.Operation == v1.Create + isUpdate = r.Operation == v1.Update + ) + src := &flux.OCIRepository{} if err := json.Unmarshal(r.Object.Raw, &src); err != nil { return nil, fmt.Errorf(lang.ErrUnmarshal, err) @@ -45,19 +53,12 @@ func mutateOCIRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster src.Spec.Reference = &flux.OCIRepositoryRef{} } - // If we have a semver we want to continue since we wil still have the upstream tag + // If we have a semver we want to continue since we will still have the upstream tag // but should warn that we can't guarantee there won't be collisions if src.Spec.Reference.SemVer != "" { l.Warn("Detected a semver OCI ref, continuing but will be unable to guarantee against collisions if multiple OCI artifacts with the same name are brought in from different registries", "ref", src.Spec.Reference.SemVer) } - if src.Labels != nil && src.Labels["zarf-agent"] == "patched" { - return &operations.Result{ - Allowed: true, - PatchOps: []operations.PatchOperation{}, - }, nil - } - zarfState, err := cluster.LoadZarfState(ctx) if err != nil { return nil, err @@ -74,37 +75,51 @@ func mutateOCIRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster "name", src.Name, "registry", registryAddress) - ref := src.Spec.URL - if src.Spec.Reference.Digest != "" { - ref = fmt.Sprintf("%s@%s", ref, src.Spec.Reference.Digest) - } else if src.Spec.Reference.Tag != "" { - ref = fmt.Sprintf("%s:%s", ref, src.Spec.Reference.Tag) - } - - patchedSrc, err := transform.ImageTransformHost(registryAddress, ref) - if err != nil { - return nil, fmt.Errorf("unable to transform the OCIRepo URL: %w", err) - } - - patchedRefInfo, err := transform.ParseImageRef(patchedSrc) - if err != nil { - return nil, fmt.Errorf("unable to parse the transformed OCIRepo URL: %w", err) - } + patchedURL := src.Spec.URL patchedRef := src.Spec.Reference - patchedURL := helpers.OCIURLPrefix + patchedRefInfo.Name + // Check if this is an update operation and the hostname is different from what we have in the zarfState + // NOTE: We mutate on updates IF AND ONLY IF the hostname in the request is different than the hostname in the zarfState + // NOTE: We are checking if the hostname is different before because we do not want to potentially mutate a URL that has already been mutated. + if isUpdate { + zarfStateAddress := helpers.OCIURLPrefix + registryAddress + isPatched, err = helpers.DoHostnamesMatch(zarfStateAddress, src.Spec.URL) + if err != nil { + return nil, fmt.Errorf(lang.AgentErrHostnameMatch, err) + } + } - if patchedRefInfo.Digest != "" { - patchedRef.Digest = patchedRefInfo.Digest - } else if patchedRefInfo.Tag != "" { - patchedRef.Tag = patchedRefInfo.Tag + // Mutate the oci repo URL if necessary + if isCreate || (isUpdate && !isPatched) { + if src.Spec.Reference.Digest != "" { + patchedURL = fmt.Sprintf("%s@%s", patchedURL, src.Spec.Reference.Digest) + } else if src.Spec.Reference.Tag != "" { + patchedURL = fmt.Sprintf("%s:%s", patchedURL, src.Spec.Reference.Tag) + } + + patchedSrc, err := transform.ImageTransformHost(registryAddress, patchedURL) + if err != nil { + return nil, fmt.Errorf("unable to transform the OCIRepo URL: %w", err) + } + + patchedRefInfo, err := transform.ParseImageRef(patchedSrc) + if err != nil { + return nil, fmt.Errorf("unable to parse the transformed OCIRepo URL: %w", err) + } + + patchedURL = helpers.OCIURLPrefix + patchedRefInfo.Name + + if patchedRefInfo.Digest != "" { + patchedRef.Digest = patchedRefInfo.Digest + } else if patchedRefInfo.Tag != "" { + patchedRef.Tag = patchedRefInfo.Tag + } } l.Debug("mutating the Flux OCIRepository URL to the Zarf URL", "original", src.Spec.URL, "mutated", patchedURL) - - patches := populateOCIRepoPatchOperations(patchedURL, zarfState.RegistryInfo.IsInternal(), patchedRef) - + patches = populateOCIRepoPatchOperations(patchedURL, zarfState.RegistryInfo.IsInternal(), patchedRef) patches = append(patches, getLabelPatch(src.Labels)) + return &operations.Result{ Allowed: true, PatchOps: patches, diff --git a/src/internal/agent/hooks/flux-ocirepo_test.go b/src/internal/agent/hooks/flux-ocirepo_test.go index b15fd73d3c..a7d5447e51 100644 --- a/src/internal/agent/hooks/flux-ocirepo_test.go +++ b/src/internal/agent/hooks/flux-ocirepo_test.go @@ -38,25 +38,6 @@ func TestFluxOCIMutationWebhook(t *testing.T) { t.Parallel() tests := []admissionTest{ - { - name: "should not mutate when agent patched", - admissionReq: createFluxOCIRepoAdmissionRequest(t, v1.Update, &flux.OCIRepository{ - ObjectMeta: metav1.ObjectMeta{ - Name: "already-patched", - Labels: map[string]string{ - "zarf-agent": "patched", - }, - }, - Spec: flux.OCIRepositorySpec{ - URL: "oci://ghcr.io/stefanprodan/manifests/podinfo", - Reference: &flux.OCIRepositoryRef{ - Tag: "6.4.0", - }, - }, - }), - patch: nil, - code: http.StatusOK, - }, { name: "bad oci url", admissionReq: createFluxOCIRepoAdmissionRequest(t, v1.Update, &flux.OCIRepository{ @@ -187,6 +168,92 @@ func TestFluxOCIMutationWebhook(t *testing.T) { }, code: http.StatusOK, }, + { + name: "should not mutate URL if it has the same hostname as Zarf state", + admissionReq: createFluxOCIRepoAdmissionRequest(t, v1.Update, &flux.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mutate-this", + }, + Spec: flux.OCIRepositorySpec{ + URL: "oci://127.0.0.1:31999/stefanprodan/manifests/podinfo", + Reference: &flux.OCIRepositoryRef{ + Tag: "6.4.0-zarf-2823281104", + }, + }, + }), + patch: []operations.PatchOperation{ + operations.ReplacePatchOperation( + "/spec/url", + "oci://127.0.0.1:31999/stefanprodan/manifests/podinfo", + ), + operations.AddPatchOperation( + "/spec/secretRef", + fluxmeta.LocalObjectReference{Name: config.ZarfImagePullSecretName}, + ), + operations.ReplacePatchOperation( + "/spec/ref/tag", + "6.4.0-zarf-2823281104", + ), + operations.ReplacePatchOperation( + "/metadata/labels", + map[string]string{ + "zarf-agent": "patched", + }, + ), + }, + code: http.StatusOK, + }, + { + name: "should not mutate URL if it has the same hostname as Zarf state internal repo", + admissionReq: createFluxOCIRepoAdmissionRequest(t, v1.Update, &flux.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mutate-this", + }, + Spec: flux.OCIRepositorySpec{ + URL: "oci://10.11.12.13:5000/stefanprodan/charts", + Reference: &flux.OCIRepositoryRef{ + Digest: "sha256:6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b", + }, + }, + }), + patch: []operations.PatchOperation{ + operations.ReplacePatchOperation( + "/spec/url", + "oci://10.11.12.13:5000/stefanprodan/charts", + ), + operations.AddPatchOperation( + "/spec/secretRef", + fluxmeta.LocalObjectReference{Name: config.ZarfImagePullSecretName}, + ), + operations.ReplacePatchOperation( + "/metadata/labels", + map[string]string{ + "zarf-agent": "patched", + }, + ), + }, + svc: &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "zarf-docker-registry", + Namespace: "zarf", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + Ports: []corev1.ServicePort{ + { + NodePort: int32(31999), + Port: 5000, + }, + }, + ClusterIP: "10.11.12.13", + }, + }, + code: http.StatusOK, + }, } ctx := context.Background()