Skip to content

workflow: use docker-env for building image (to ensure golang version)#223

Closed
srenatus wants to merge 1 commit intoopen-policy-agent:masterfrom
srenatus:sr/pipeline/ensure-container-is-built-with-proper-golang-version
Closed

workflow: use docker-env for building image (to ensure golang version)#223
srenatus wants to merge 1 commit intoopen-policy-agent:masterfrom
srenatus:sr/pipeline/ensure-container-is-built-with-proper-golang-version

Conversation

@srenatus
Copy link
Collaborator

Before, the image would invoke build-linux, and that wouldn't guarantee the
version of golang used for building.

Now, the image target goes through a round of make release to ensure just
that.

As a consequence, we'll build windows and darwin binaries where we don't need
them, but let's see if the time wasted is problematic at all.

To verify that this does what it's supposed to do, I've overridden the VERSION
makefile variable, and ran make image. The resulting container reports:

$ docker run openpolicyagent/opa:"0.24.0-envoy-11" version
Version: 0.24.0-envoy-11
Build Commit: e4f19267
Build Timestamp: 2020-11-30T10:40:36Z
Build Hostname: 6bec0a11f7e7
Go Version: go1.15.2

Fixes open-policy-agent/opa#2942.


⚠️ There's one thing to note here -- actually running the service built using that Golang version means it'll be less tolerant towards TLS certs than it was before. In #217, I've run into this locally, so the examples/istio/quick_start.yaml certs have been adapted, but there hasn't been a release of opa-envoy-plugin using Go 1.15.x until we ship this change.

For more details about what's going on, please refer to the release notes:

The deprecated, legacy behavior of treating the CommonName field on X.509 certificates as a host name when no Subject Alternative Names are present is now disabled by default. It can be temporarily re-enabled by adding the value x509ignoreCN=0 to the GODEBUG environment variable.

Note that if the CommonName is an invalid host name, it's always ignored, regardless of GODEBUG settings. Invalid names include those with any characters other than letters, digits, hyphens and underscores, and those with empty labels or trailing dots.

Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment about permissions on the binary.

RE: the cert/go version issue: I think this is fine. I'm assuming this only applies to the certs in the quick_start.yaml file that are used for the admission controller (which is basically an installer for an example...)

I'm tempted to put a comment in the quick_start.yaml file that indices we don't guarantee backwards compatibility on the quick_start.yaml manifest (although that might get misinterpreted...)

Makefile Outdated
image:
@$(MAKE) build-linux
image: release
mv $(RELEASE_DIR)/opa_envoy_linux_$(GOARCH) .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make release may produce a binary owned by root since it was built inside of Docker...perhaps we could just modify the Dockerfile to copy from the release directory instead of the root directory--that would avoid the mv.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do that, the path I started on led to a mess, though. I'll clean it up tomorrow. We don't need multiple Dockerfiles, there's only one thing built here. 🤔

@srenatus srenatus marked this pull request as draft November 30, 2020 19:45
@srenatus srenatus force-pushed the sr/pipeline/ensure-container-is-built-with-proper-golang-version branch 2 times, most recently from 0a07e51 to 0946a7b Compare December 8, 2020 13:48
@srenatus srenatus marked this pull request as ready for review December 8, 2020 13:48
@srenatus srenatus marked this pull request as draft December 8, 2020 19:27
@srenatus
Copy link
Collaborator Author

srenatus commented Dec 8, 2020

I'll fix e2e tomorrow, it'll be affected by #202 getting merged anyways, I suppose.

Before, the `image` would invoke `build-linux`, and that wouldn't guarantee the
version of golang used for building.

Now, we're following the schema used by OPA to build the binary into a release dir,
and have the image build pick it up.

Also, the built image now includes everything needed to use wasm:

    $ docker run  -it docker.io/openpolicyagent/opa:0.25.1-envoy-2 version
    Version: 0.25.1-envoy-2
    Build Commit: 20832273
    Build Timestamp: 2020-12-08T13:34:55Z
    Build Hostname: ce578c1a1309
    Go Version: go1.15.2
    WebAssembly: available
    $ docker run --entrypoint busybox -it docker.io/openpolicyagent/opa:0.25.1-envoy-2 sh
    / # ls /usr/lib/opa
    libwasmer.so
    / #

We do this by building off of the openpolicyagent/opa package, which has the
library in place.

Fixes open-policy-agent/opa#2942.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@srenatus srenatus force-pushed the sr/pipeline/ensure-container-is-built-with-proper-golang-version branch from 0946a7b to 2b1b571 Compare December 8, 2020 19:36
@srenatus srenatus closed this Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image 'openpolicyagent/opa:0.24.0-envoy-11' is not built with the declared golang version (1.15.2)

2 participants