grafana / rollout-operator

Kubernetes Rollout Operator
Apache License 2.0
132 stars 17 forks source link

Add boringcrypto image #71

Closed dimitarvdimitrov closed 1 year ago

dimitarvdimitrov commented 1 year ago

This adds a boringcrypto image

dimitarvdimitrov commented 1 year ago

The CI is failing the same bug that @andyasp encountered here https://github.com/grafana/rollout-operator/pull/70#issuecomment-1634544728

dimitarvdimitrov commented 1 year ago

thanks for the reviews @aknuds1, I think I addressed all of the issues. Sorry for the state of this PR; I should have reviewed it on my own a few times before opening.

dimitarvdimitrov commented 1 year ago

I pushed a commit to pin the go version so we can see a passing CI. After that I'll undo the pin and merge on failing CI

dimitarvdimitrov commented 1 year ago

I'm investigating the failing CI

dimitarvdimitrov commented 1 year ago

I had to make some adjustment to the PR. As of now this is the amd4 binary built from this PR using the commit where CI tests ran - 7a0770d

$ file /tmp/rollout-operator
/tmp/rollout-operator: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, Go BuildID=GpWoX651AkwtQPsrgVNx/nuEX87cG0CpHr_FAQAlZ/WGnquJfCp-utqUX_5FNc/eePK3vlRL3Hn65mybSiG, with debug_info, not stripped

and includes boringcrypto & FIPS symbols as opposed to standard library symbols

$ go tool nm /tmp/rollout-operator | grep FIPS
  461b80 t BORINGSSL_FIPS_abort
  45da80 t FIPS_mode_set
  45da90 t FIPS_read_counter
  401490 T _cgo_d8b1bdd8e714_Cfunc__goboringcrypto_FIPS_mode
  45da70 t _goboringcrypto_FIPS_mode
  65bd40 T crypto/internal/boring._Cfunc__goboringcrypto_FIPS_mode.abi0
 2a89888 D crypto/internal/boring._cgo_d8b1bdd8e714_Cfunc__goboringcrypto_FIPS_mode
  5eadc0 T crypto/internal/boring/sig.FIPSOnly.abi0
 2a8ae30 D crypto/tls.defaultCipherSuitesFIPS
 2a8aeb0 D crypto/tls.defaultFIPSCurvePreferences

The tests pass on https://github.com/grafana/rollout-operator/pull/71/commits/7a0770dea6f54c50328075874062f9ec3a3e26b2 with the go version pinned to 1.20.5. I reverted the pin in f4ef221a1041c8f073fb0c67bd9b368bacf23e99 that's why the branch is now failing CI. The failing CI is due to a bugfix in docker which hasn't reached us yet (see https://github.com/grafana/rollout-operator/pull/70#issuecomment-1634544728)

dimitarvdimitrov commented 1 year ago

commands ran on the image from https://github.com/grafana/rollout-operator/pull/71/commits/617719f28ff173406c89b8ca97e3b1bb7d646d53

file /bin/rollout-operator
/bin/rollout-operator: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-musl-x86_64.so.1, Go BuildID=QhjtoKw-eV539V2keY8t/JMinCisYZsWEMlAzPP4y/VTEVI2hm4Ymq6TZTkxhX/fQgtBavrsbgLnofGYkki, with debug_info, not stripped
ldd /bin/rollout-operator
    /lib/ld-musl-x86_64.so.1 (0x7f1839c4f000)
    libc.musl-x86_64.so.1 => /lib/ld-musl-x86_64.so.1 (0x7f1839c4f000)
go tool nm /tmp/rollout-operator | grep -E 'sig.FIPSOnly|sig.BoringCrypto|sig.StandardCrypto'
  5ebda0 T crypto/internal/boring/sig.BoringCrypto.abi0
  5ebdc0 T crypto/internal/boring/sig.FIPSOnly.abi0
aknuds1 commented 1 year ago

For context, @dimitarvdimitrov and I decided to not statically link the boringcrypto based rollout-operator, since it leads to warnings during the link stage about implicit libc dependencies at runtime (also glibc static linking is discouraged). The dynamically linked image should work perfectly within the Docker image, as it just links to Alpine's musl (libc) library.