Skip to content

Commit

Permalink
fix: maintain agent mutate even when already mutated (#3166)
Browse files Browse the repository at this point in the history 
Signed-off-by: Allen Conlon <[email protected]>
  • Loading branch information
@a1994sc
a1994sc authored Nov 21, 2024
1 parent 2e66463 commit a4898df
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 77 deletions.
48 changes: 33 additions & 15 deletions src/internal/agent/hooks/flux-helmrepo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ 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)

src := &flux.HelmRepository{}
if err := json.Unmarshal(r.Object.Raw, &src); err != nil {
return nil, fmt.Errorf(lang.ErrUnmarshal, err)
Expand All @@ -48,13 +49,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 +64,45 @@ 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

var (
isPatched bool

isCreate = r.Operation == v1.Create
isUpdate = r.Operation == v1.Update
)

// 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())
var patches []operations.PatchOperation

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
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
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

0 comments on commit a4898df

Please sign in to comment.