cert-manager / aws-privateca-issuer

Addon for cert-manager that issues certificates using AWS ACM PCA.
Apache License 2.0
184 stars 77 forks source link

feat: linux/arm64 compatibility #319

Closed isometry closed 2 months ago

kollabpr commented 4 months ago

Hi @isometry, Thanks for the feature request and the PR! We will review with the team and get back to you soon.

dcamzn commented 4 months ago

Thank you. We plan to support ARM this year, however, it doesn't look like we can deliver ARM support in Q2.

isometry commented 4 months ago

Thank you. We plan to support ARM this year, however, it doesn't look like we can deliver ARM support in Q2.

What more is needed beyond the changes in this PR? Can we help?

jtackaberry commented 3 months ago

What more is needed beyond the changes in this PR?

I imagine they might want a better integrated solution? Your PR understandably makes efforts to be non-invasive and low risk (by creating a temporary hacked version of the Dockerfile), but IMO a proper solution would just have a working Dockerfile in all cases.

In any case @isometry I tried your patch but the arm64 images aren't actually working on arm64. Classic "exec format error" on execution.

This is because of https://github.com/docker/buildx/issues/510 and I needed to make this change to the Dockerfile to get a proper build:

diff --git a/Dockerfile b/Dockerfile
index 7288a6b..533705f 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -2,8 +2,8 @@
 FROM golang:1.21 as builder
 WORKDIR /workspace

-ARG TARGETARCH=amd64
-ARG TARGETOS=linux
+ARG TARGETARCH
+ARG TARGETOS

 ENV GOPROXY=direct
 # Copy the Go Modules manifests
@@ -18,8 +18,8 @@ COPY main.go main.go
 COPY pkg/ pkg/

 ENV CGO_ENABLED=0
-ENV GOOS=$TARGETOS
-ENV GOARCH=$TARGETARCH
+ENV GOOS=${TARGETOS:-linux}
+ENV GOARCH=${TARGETARCH:-amd64}
 ENV GO111MODULE=on
isometry commented 3 months ago

Thanks @jtackaberry! The Dockerfile hack is what operator-sdk is currently pushing (IIRC). I personally favour building a minimalist image with ko. My initial local patch actually included your fixes but I reverted in a misguided attempt to minimise the footprint of the patch after seeing other operators using the original pattern. Oh well. I'll update :-)

isometry commented 3 months ago

@jtackaberry : as per your suggestion, I've tidied up to remove unnecessary shenanigans. The result is available at ghcr.io/isometry/aws-privateca-issuer:latest for testing.

cert-manager-prow[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/cert-manager/aws-privateca-issuer/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment