Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: maintain agent mutate even when already mutated #3166

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: maintain agent mutate even when already mutated #3166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d75c6b1
fix: maintain agent mutate even when already mutated
a1994sc Oct 31, 2024
edd3b56
Merge branch 'main' into main
a1994sc Nov 7, 2024
b8e9f77
Updated logic for when to mutate
a1994sc Nov 8, 2024
a93f280
Merge remote-tracking branch 'origin-zarf/main'
a1994sc Nov 9, 2024
db8c955
Merge branch 'main' into main
a1994sc Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions src/internal/agent/hooks/flux-helmrepo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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{
Expand Down
85 changes: 74 additions & 11 deletions src/internal/agent/hooks/flux-helmrepo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
},
}),
Expand Down
79 changes: 47 additions & 32 deletions src/internal/agent/hooks/flux-ocirepo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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,
Expand Down
Loading