jetstack-secure icon indicating copy to clipboard operation
jetstack-secure copied to clipboard

🔥 Migrating to Makefile Modules ALL AT ONCE 🔥

Open maelvls opened this issue 1 year ago • 1 comments
trafficstars

I've revived https://github.com/jetstack/jetstack-secure/pull/555. Here is why I don't think we should do gradually:

  1. The old make build-docker-images was running a make command within a container in a buildx env, which made things super complicated for nothing
  2. Building in a container using buildx/QEMU isn't needed, slows us down. If we had CGO_ENABLED=1 set, I would understand. But since we don't, we should Ko.
  3. I don't think the existing Makefile was doing much anyways.

This PR is my ongoing effort to fully migrate this repo to makefile-modules.

A few things are already working:

make helm-chart
make oci-push-preflight
# To generate jetstack.io_venaficonnections.yaml:
make generate-manifests

I haven't tested any of it, especially the compatibility old/new regarding the image and Helm chart.

Notes:

  • I removed the ldflags version.Platform and version.GoVersion as these are already available from the "runtime" package.

maelvls avatar Aug 13 '24 20:08 maelvls

I will continue this on Monday, Aug 19th if that's OK. I really enjoy working with Makefile Modules.

maelvls avatar Aug 13 '24 20:08 maelvls

Thanks for the review, Tim. For context, I paused working on this because I have been focusing on VC-35568 which (I was told) is higher priority.

I want to resume working on this as soon as I make some progress on VC-35568. Although I'd love to continue working on this, I think it can wait one week or two. WDYT?

maelvls avatar Sep 12 '24 06:09 maelvls

@maelvls I think the PR is ready to be merged. In follow-up PRs, we can:

  • rename "github.com/jetstack/preflight" to "github.com/jetstack/jetstack-secure" and "preflight" to "agent"
  • remove old unused files
  • fix linter errors
  • add e2e test

inteon avatar Sep 30 '24 13:09 inteon

Thanks for working on the GitHub Actions. I've looked at them and they look OK. Let's merge this and try releasing v1.1.0-alpha.0 to see if everything works.

maelvls avatar Sep 30 '24 13:09 maelvls