feat: add registry1 zarf flavors to uds core

[zachariahmiller]

Description

Adds zarf flavors for upstream images and IB. Includes refactoring of zarf packages using composability to cleanup the config and keep things dry and refactoring of the values files to image flavors specific + common.

Both flavors deploy successfully on their respective architectures. On arm64 (or at least darwin arm64) when using the registry1(x86) flavor it fails at metrics server regarding the istio init container, which seems to be an issue emulating x86 on mac.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Other (security config, docs update, etc)

Checklist before merging

• Test, docs, adr added or updated as needed
• Contributor Guide Steps(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request) followed

[zachariahmiller]

I'll work on resolving the merge conflicts from the operator PR, testing bundle publish flavor behavior and setting a default flavor in the tasks tonight and tomorrow morning.

Intermittent neuvector deploy is still an issue as they dont actually expose security context in the monitor chart and it sometimes happens to work by continuing past it anyways. Not sure what the immediate fix for this is, but @mjnagel put in a pr to the upstream chart to fix on their side that we can use once it gets added. neuvector/neuvector-helm#355

[jeff-mccoy]

these skipped tests also look pretty ugly, think we need to address that as well.

[zachariahmiller]

these skipped tests also look pretty ugly, think we need to address that as well.

I dont disagree. Again, related to the upgrade. Not sure how to fix this without a more significant refactor of the workflows. IMO that could be addressed in a follow on PR, but whatever the team wants to do.

[zachariahmiller]

Left a few comments, but overall looks pretty good. Couple questions:

1. How will this work with Renovate with the mess of version tracking between IB and upstream?

2. Not sure we should be testing both flavors for single-app tests are those are really just circuit-breaker tests to speed up failure feedback.

1. The goal with the way I broke out the repo/image/tag pieces for each flavors values file was supposed to deal with that. When i was testing the renovate config i was testing with a WIP version of flavors. It was mostly working, one of the issues we had was that when image versions werent referenced at all (some of upstream) the renovate pr wouldnt always be consistent, so the current pattern is to make the registry1 anbd upstream image specific pieces match completely so we shouldnt have any weirdness with renovate. I think we should be in a pretty good place as far as that goes and we can always tweak the renovate config as needed. Most of them should update correctly grouped by app including both flavors. I'm sure there will be some exceptions (wouldnt be surprised if something takes an issue with this "registry.k8s.io/ingress-nginx/kube-webhook-certgen:v20221220-controller-v1.5.1-58-g787ea74b6" for example :D )

2. Maybe we dont, but those tests are free. There was a discussion earlier in the review on that feat: add registry1 zarf flavors to uds core #63 (comment) i think given it is no cost to us with some potential minor gain (aside from whether we chose to test registry1 or upstream) there isnt really a compelling reason to only test one. As it stands though if we did that, which is what i originally implemented, we'd end up with the same skipped test thing you se now for the upgrades, which would require refactoring the workflow files more.

[zachariahmiller]

@jeff-mccoy here is an example of what we expect the renovate updates to look like more or less. This was in my fork before making the registry1 and upstream values files both have all the image refs rather than relying on it just working in upstream. With the values file changes it will update the upstream-values, registry1-values, and the zarf.yaml images and chart versions in one PR.

[zachariahmiller]

For clarity/transparency we were trying to determine why the fast failure started only working on a click ops cancel and not a job failure in the updated ci.

It looks like we have two reasonable options to get it back to that functionality.

Option 1:
Use something like this composite action but built on bash/sh not powershell https://github.com/Lombiq/GitHub-Actions/blob/dev/.github/actions/cancel-workflow/action.yml#L27 This should work but it looks like from @mjnagel research there are some downsides:

This is the simplest way I've seen to cancel other jobs. But the downside of that is that it results in everything showing cancelled so it's a bit harder to find the failed job...

Option 2: Go back to the previous workflow setup (all calling just the one test workflow) while expanding the matrix. I tested/mocked this in an outdated fork as it was the easiest way to do so and it seems to work, downside is that the actual display of the job names and lack of grouping is a little less readable. The mocked up version of that can be seen here:
zachariahmiller#42
https://github.com/zachariahmiller/uds-core/actions/runs/7595497465/job/20688138631?pr=42

[zachariahmiller]

@mjnagel I dug into the matrix functionality a little more. It looks like with a combination of excludes and includes in the matrix definition, relying on order they are processed per the docs seems to work. For example:

    strategy:
      matrix:
        package: ${{ fromJSON(needs.check-paths.outputs.packages) }}
        flavor: [upstream, registry1]
        test_type: [install, upgrade]
        exclude:
          - test_type: upgrade
        include:
          - test_type: upgrade
            flavor: registry1
            package: all
          - test_type: upgrade
            flavor: upstream
            package: all

Excludes the single upgrade tests and still does both for all while maintaining the fail fast functionality.

As you can see in the screenshot, the single upgrades were excluded, install and upgrade test ran for "all" and the failed job short circuited the all the sibling workflows spawned from the matrix config while still failing the pipeline.

[mjnagel]

@zachariahmiller I think that makes sense. This slight modification (if it works) would make it a bit simpler I think?

    strategy:
      matrix:
        package: ${{ fromJSON(needs.check-paths.outputs.packages) }}
        flavor: [upstream, registry1]
        test_type: [install]
        include:
          - test_type: upgrade
            flavor: registry1
            package: all
          - test_type: upgrade
            flavor: upstream
            package: all

If I read the docs right I think that'll work? May even be able to drop the flavor from the include but feel less confident on that.

[zachariahmiller]

If that works, agreed. As far as dropping flavor, I don't know if it will... With the combo of include and exclude the flavor was required. IIRC exclude worked on partial match, include didn't.

[jeff-mccoy]

🚀

[jeff-mccoy]

Branch protection updated, g2g.