Conversation
|
Tag for build #417071 is 💻 For deploying this image using the dev scripts, run the following first: export MAIN_IMAGE_TAG='3.69.x-337-ga673a416d8'🕹️ A |
gavin-stackrox
left a comment
There was a problem hiding this comment.
Less is better! But I will need to defer approval to someone that knows CGO.
msugakov
left a comment
There was a problem hiding this comment.
Not sure what ci-openshift-4-operator-tests label controls but please add it to this PR.
operator/Dockerfile
Outdated
|
|
||
| # Build the operator binary. | ||
| RUN GOOS=$(go env GOOS) scripts/go-build.sh operator | ||
| RUN GOOS=$(go env GOOS) CGO_ENABLED=0 scripts/go-build.sh operator |
There was a problem hiding this comment.
Disabling CGO looks rather arbitrary. This will create a mismatch between upstream (this) and downstream builds where CGO is not disabled. Having such mismatch can lead to different behavior which isn't good because downstream images are not as thoroughly tested end-to-end as in upstream CI.
Last time I checked with Malte, there were specific reasons why CGO was enabled. I don't want to be a broken telephone relaying that info, particularly because I will have to research it myself. I believe there's more than "just" rocksdb, there are some tracing concerns that customers have.
Could you please search around for considerations why main and other images are built with CGO?
There was a problem hiding this comment.
In main we need rocksb to be linked. Here we don't have any external C libs so we don't need CGO.
The only one drawback I found is when disabling CGO we end up using go network stack and DNS may behave differently.
There was a problem hiding this comment.
Thanks Tomek but I don't think this info is sufficient. Have you read what I wrote just above?
Last time I checked with Malte, there were specific reasons why CGO was enabled. I don't want to be a broken telephone relaying that info, particularly because I will have to research it myself. I believe there's more than "just" rocksdb, there are some tracing concerns that customers have.
Re-stating the same thing. CGO allows customers better trace the code execution because they can attach certain tools to binaries when they are built with CGO. This is real requirement from real StackRox customers.
I suggest you to research more on this topic. Either of two outcomes are possible:
- This is still the requirement and we still have to do it for the operator.
- This is not a requirement any more and we can stop doing that.
There was a problem hiding this comment.
Got it. I'll find answer about this requirement. For now... how about ubi8-micro?
There was a problem hiding this comment.
Thanks Tomek, sounds good.
d645ca3 to
eca3156
Compare
| # The following are numeric uid and gid of `nobody` user in UBI. | ||
| # We can't use symbolic names because otherwise k8s will fail to start the pod with an error like this: | ||
| # Error: container has runAsNonRoot and image has non-numeric user (nobody), cannot verify user is non-root (pod: "stackrox-operator-controller-manager-75bc744454-bkbjr_stackrox-operator-system(49874aae-2695-4d3a-afd3-8723914d2af5)", container: manager) | ||
| USER 65534:65534 |
There was a problem hiding this comment.
@msugakov Why do we need that? I can add /etc/passwd to the image but I'm not sure if that's necessary
There was a problem hiding this comment.
You don't need /etc/passwd. When leveraging numeric uid:gid, the user doesn't need to exist in /etc/passwd.
If USER is not specified, the container will run as root. Maybe we override this in Kubernetes manifests, but still it is good to have run-as-user defined in the Dockerfile.
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
Description
Currently we are using
ubi8-minimalimage but we only put operator there that could be build without CGO.Operator should not require any other binaries or libs so there is no need to use system image and we can use
scratchimage.Results
Checklist
If any of these don't apply, please comment below.
Testing Performed
CI