In T334254#8772309, @dcausse wrote:

• which is I think executed within the arm64 image and then are causing the error: standard_init_linux.go:219: exec user process caused: exec format error. My understanding is that if I want to use copies I must be setting lives.in, in my cases all the folders/users are existing, would there be a way to skip this detection and tell blubber to trust my image?

@dcausse thanks for filing this.

Confirmed that we can now push blubber's multi platform image to our registry. See https://gitlab.wikimedia.org/repos/releng/blubber/-/jobs/90258

I was able to reproduce the problem locally and it has to do with how nginx (doesn't) apply the set $auth_request_path to the nested location block that matches exact manifest/blob digests. From the commit message of the associated patch:

Thank you, @JMeybohm. That's very helpful.

Friendly ping. :)

I'm finally circling back to this, and I ran another test yesterday. The additional logging in jwt-authorizer confirms that this is an auth failure due to the token scope not matching the request URL. However, the reason for this mismatch is strange. (See below.)

The last few rounds of load testing, whereby 100 concurrent image-build jobs are triggered at once, have all been successful in that:

Just noting that the specific CPS mismatch here is the use of a CPS transformed method in a constructor. Constructors are never CPS transformed.

Deployed.

I did some more investigation on this to better understand how exactly Istio is interfering with the traffic from the network namespaces on the buildkit0 bridge interface, and in doing so discovered the traffic.sidecar.istio.io/kubevirtInterfaces annotation that solves this problem in a less hacky way—or at least in an official Istio hacky way as the resulting iptables PREROUTING chain looks similar to what we added.

Awesome!

BuildKit has been redeployed. It now runs in privileged mode and has CNI configured for network isolation of build containers.

For starters, perhaps we can create an account on docker.io for Release Engineering and generate a public puller token to use on WMCS runners. According to the GitLab CI runner docs, the docker executor should respect the ~/.docker/config.json for the runner's user. In this scenario, the creds would not be leaked to jobs.

Thanks very much for your help today, @cscott and @ssastry. I've re-rolled group1.

Thanks! I will merge and deploy the backport. I might wait until tomorrow to re-roll train to group1, depending on my own time constraints.

In T325586#8620165, @ssastry wrote:

Let me consult with Scott and we'll respond here with a proposal.

In T325586#8620127, @ssastry wrote:

So, in terms of decision, what we need to figure out is whether we suppress logs as in bullet point #1 OR if we should try to fix the code and try to get it on the train. Since this is in the Parsoid repo, it is more involved and requires a new tag, vendor release, patch to update vendor in core, etc.

In T325586#8620127, @ssastry wrote:

Ya, so, 3 things:

• Suppressing notices as in T329740#8618948 gets rid of the noise (and suppresses the spike)
• The other errors Scott referenced and one I was concerned about are only a handful and seem to be seen during rollback (afaict) and are transient ones.
• The UTF-8 errors are also all a handful and all old known things which could potentially be bad wikitext being posted -- nothing specific to this train.

So, in terms of decision, what we need to figure out is whether we suppress logs as in bullet point #1 OR if we should try to fix the code and try to get it on the train. Since this is in the Parsoid repo, it is more involved and requires a new tag, vendor release, patch to update vendor in core, etc.

@cscott I'm seeing a large spike in errors from Parsoid today. See https://logstash.wikimedia.org/goto/a7eb8ccc8cf7a123b9577e32e98f1b1c

Summary of our current solution:

I should clarify. I did not refactor the security patch implementation, only the context so it would apply cleanly.

I've refactored the patch following application failure during scap stage-train.

In T329266#8606347, @sbassett wrote:

Hey @dduvall - Thanks for looking into this! +1 to your patch above and yes, it should be fine to push through gerrit since the "secret" static analysis script has been public for a long time :)

Patch file for integration/config ^. I assume it should be fine to submit this for review since I've already updated the job in Jenkins, but I thought I'd check with you first, @sbassett.

I've updated the job from changes I made locally and it seems to work as expected.

Given this job is run inside a container as user nobody (which is the cause of the error since the repo is mounted and has different ownership), I think it's probably safe to add git config --global --add safe.directory to the script, but the checks (the entire script really) seem quite difficult to reason about and the status of the git command should be handled separately from those of sed/grep.

This is done. Evaluation has long been done and we have a working image build system that can also publish (and soon will perform test deployments).

In T316519#8532670, @Ottomata wrote:

Should we change this? Should we set the runs.as to something different when building images based of of the production-images flink image with blubber?

In T322344#8484468, @MoritzMuehlenhoff wrote:

You would have needed that with the previous configuration as well, the updates config only makes this easier on the SRE end. I have setup a systemd-nspawn container which has the terraform source config. So when we need an update I can simply run "apt-get download terraform" and import the deb on apt1001.

(Orthogonal but worth a discussion in our next meeting.)

In T322344#8473349, @MoritzMuehlenhoff wrote:

In T322344#8471866, @dduvall wrote:

I'm thinking the best course for us at this point might be to rely on binaries verified by checksum for now and not the upstream deb package.

Nah, let's not do that. thirdparty/terraform was missing because the older config caused issues with reprepro and had been reverted. I have now merged a patch to readd thirdparty/terraform and import terraform 1.3.6, please give your image build another shot.

I'm thinking the best course for us at this point might be to rely on binaries verified by checksum for now and not the upstream deb package.

We're now seeing errors during our image build. It seems the component and package may no longer be in our repo.

One more thought: Since the DigitalOcean runners will be instance wide starting in the new year and will be configured to run untagged jobs, I'm not sure their tags will matter as much for the most general use cases.

In T325069#8470932, @Jelto wrote:

In T325069#8468648, @bd808 wrote:

I know that I am a leading contributor to this problem, but "cloud" is pretty closely tied to Wikimedia Cloud Services within the technical side of the movement. Could we find a different keyword to use to describe the untrusted runners?

I opened https://gitlab.wikimedia.org/repos/releng/gitlab-cloud-runner/-/merge_requests/92. See also my comment in the MR. I have some concerns of using a provider specific tag because that means end-users have to change CI configurations when we switch to a different provider. After thinking a bit more about tagging for public cloud runners I would like to keep use cloud as the primary tag. I think cloud is the best current description for public cloud providers without being too technical to end users. Furthermore the term/tag cloud was already announced in the latest monthly Tech Department Updates.

I understand that people with more history and time in the foundation may have a different opinion about that, especially because there was no other "cloud" at that time.

We can add a second, provider specific tag like digitalocean for easier maintenance.

I would vote for

Trusted runners: trusted
Shared runners in public cloud: cloud, kubernetes, optional digitalocean
Shared runners in WMCS: wmcs

Which way are we going with this? FWIW I think fronting with nginx would allow offloading of both auth (via jwt-authorizer) and GET/HEAD request caching. Honestly, if we could get the existing nginx ingress working for internal requests, it could handle all of that.

It's possible for a blubber.yaml to be in a subdirectory of .pipeline as well. Could you run the command again with a less restrictive pattern, maybe just grep -iq blubber.yaml?

Paired with @dancy and @jeena on this today.

Done. See T322691#8433642

@Jelto or @Dzahn we'll need this built for buster as well since the registry hosts are all buster based.

The buildkitd deployment running in cloud-runner now has access to the DO Spaces credentials. However, there's a pretty large snafu here that I ran into while testing with a blubber MR.

I recently enabled the prometheus metrics exporter and prometheus operator in the gitlab-runner chart.

In T322344#8385118, @Dzahn wrote:

@dduvall It's available for bullseye now:

[apt1001:/tmp] $ sudo -E reprepro ls terraform
terraform | 1.3.4 | bullseye-wikimedia | amd64

\o/ thank you!!!

In T322344#8385115, @Dzahn wrote:

@dduvall The way you describe above works but it seems to me that is simply because it skips the "signed by" line in APT sources and does not verify. It would also work if you skip the installation of the gpg key entirely.

Would it be ok to add GetInRelease: no to the reprepro updates file? According to the manpage:

In T322344#8384985, @Dzahn wrote:

I opened a ticket with upstream. (#89323)

In T322344#8384171, @MoritzMuehlenhoff wrote:

Hmmh, this is quite strange. There's no obvious error in the config (but the claimed signed on 1970 may be the culprit) and the key has been imported correctly on apt1001. This will need some further debugging, I'll try to get to it in the next days, currrently quite busy.

@Dzahn In the mean time to unblock the work, could you fetch the deb via secure apt (by setting it up following the upstream docs to setup the apt source and then running "apt-get download terraform" and then importing the package using "reprepro includedeb"?

I enabled debug logging for buildkitd on the gitlab-runner hosts and re-ran the failed job. It appears that auth failure may be the root cause—the client is definitely misbehaving but the there should not be auth failure here. It's unclear why.

@JMeybohm can you provide the nginx access log entries from that time period as well? I'm trying to rule out auth failure as a factor and docker-registry log entries do not include the subrequests between nginx and jwt-authorizer.

In T322579#8378513, @JMeybohm wrote:

In T322579#8378397, @Joe wrote:

[...]
Also: both the registry and nginx keep access logs, so I guess it's enough to export one of the two.

I'd say export those from docker-registry. It writes access log plus additional stuff that might be useful

Thanks for debugging this further, @JMeybohm and @hashar. In your recent re-run of the job it seems the manifest list is accepted by the registry, and it's the subsequent manifest push that fails, so you're right this doesn't seem to be related to a lack of manifest list support for perhaps some errant behavior on buildkit's side that manifests under this multi-platform push condition. I'll re-word and triage accordingly.

In T322344#8377390, @Dzahn wrote:

...
ERROR: Condition 'DA418C88A3219F7B' not fulfilled for '/srv/wikimedia/lists/thirdparty%2Fterraform-bullseye_bullseye_InRelease'.
Signatures in '/srv/wikimedia/lists/thirdparty%2Fterraform-bullseye_bullseye_InRelease':
'DA418C88A3219F7B' (signed 1970-01-01): bad signature
Error: Not enough signatures found for remote repository thirdparty/terraform-bullseye (https://apt.releases.hashicorp.com bullseye)!
There have been errors!

Thanks for the review/merge, @MoritzMuehlenhoff and @Dzahn! I don't see the packages yet but I'm assuming the actual import is a manual step?

In T318382#8317695, @dduvall wrote:

Thanks, Moritz. Can we do the same for buster, pulling in the newer 20.10.18~3~debian-buster packages as well? I think using the same exact version on contint and integration agents is probably best.

Thanks for filing this!

In T322453#8375676, @JMeybohm wrote:

I think this might require an update of our docker-registry (which we have not planned for currently). Is this blocking something apart from testing with multi arch images (T272500)?

In T321316#8369939, @Jelto wrote:

In the last IC sync meeting we discussed that it makes more sense to add the self-hosted dockerfile/copy image to production-images first instead of hosting that separately in GitLab.

This may be blocked by T322453: Buildkit erroring with "cannot reuse body, request must be retried" upon multi-platform push which is preventing the publishing of multi-platform images. The dockerfile-copy image needs to be multi-platform to achieve parity with the upstream version.

See https://gitlab.wikimedia.org/repos/releng/blubber/-/merge_requests/15 for multi-platform support in Blubber, currently under review.

In T318866#8360111, @bd808 wrote:

Reading T321316: Self-build and publish buildkit helper images clued me into the copy action where the crash is happening being very likely done via the docker/dockerfile-copy image which is used as a helper by buildkit.

In T321316#8359399, @Jelto wrote:

Thanks @Joe for the analysis! That pushed my in the right direction.

I also found the code/project for the copy binary here: https://github.com/tonistiigi/copy . There is also a Dockerfile in there.

I'll try to migrate that to a GitLab project. Best case with a job to build the copy binary, otherwise we can import it. First I have to verify if that's the correct code repository and what the copy tool is doing.