[FLINK-30921][ci] Adds mirrors instead of relying on a single source for Ubuntu packages #21873
Conversation
|
It seems to appear again. I rebased the branch and squashed the commits to make it ready to be merged in case, we're having this issue for longer again. |
XComp
changed the title
|
I added a reference to the original comment in the code. |
There was a problem hiding this comment.
@XComp Nice improvement. I'm leaving one comment for transparency but I don't consider it a blocker for this PR.
| echo -e "${default_ubuntu_mirror_url}\tpriority:1" | sudo tee ${mirror_file_path} | ||
|
|
||
| # use other mirrors as a fallback option | ||
| curl http://mirrors.ubuntu.com/mirrors.txt | sudo tee --append ${mirror_file_path} |
There was a problem hiding this comment.
The only thing this could introduce is option for a supply chain attack, but we already have others sources where we do something similar and I would classify the risk that Ubuntu gets compromised the same as we do for the other sources (Maven/Github).
There was a problem hiding this comment.
Interesting point. Just to understand that: It makes us vulnerable because we're relying on mirrors.ubuntu.com/mirrors.txt? What about specifying a fixed list of mirrors ourselves instead of relying on an external service? That should be good enough to serve our needs and be less reliant on the stability of Azure's Ubuntu mirros.
There was a problem hiding this comment.
It makes us vulnerable because we're relying on
mirrors.ubuntu.com/mirrors.txt
Yes
What about specifying a fixed list of mirrors ourselves instead of relying on an external service?
That would remove the attack vector, as long as we specify it in the repo :)
There was a problem hiding this comment.
I guess, that's reasonable. I will update the branch accordingly. Thanks for bringing it up :-)
There was a problem hiding this comment.
@MartijnVisser Any objections against that list?
|
|
||
| # This mirror list can be used in CI to set up the Docker containers without only | ||
| # relying on the default Ubuntu mirror provided by Azure. | ||
| # FYI: apt ignores lines that are empty or start with '#' |
There was a problem hiding this comment.
I did a brief check of the apt code on that one: https://github.com/Debian/apt/blob/b3a5bbe234d4206b7736bca2ca7f56feaa92f5ff/methods/mirror.cc#L242
| # primary mirror | ||
| http://azure.archive.ubuntu.com/ubuntu/ priority:1 | ||
|
|
||
| # fallback mirrors selected from http://mirrors.ubuntu.com/ based on the assumption that CI |
There was a problem hiding this comment.
Not relying on what's offered by mirrors.ubuntu.com dynamically deprives us from having a dynamic mirror list depending on the CI machines location. But that's a minor issue in my opinion because we want to rely on Azure's mirror in general and only want to prevent CI failures if Azure's mirror is not available. In the worst case, downloading the artifacts would take more time then, I guess.
|
@flinkbot run azure |
…for Ubuntu packages We don't want to rely on an external service for retrieving mirrors dynamically to prevent supply chain attacks. Therefore, we selected a few mirrors manually. This prevents us from relying on a dynamic mirror list based on the CI machines location which might be a performance issue. But generally, we want to rely on the Azure mirror anyway. The fallback list is only meant to jump in if Azure's mirror is not accessible temporarily. Signed-off-by: Matthias Pohl <matthias.pohl@aiven.io>
|
k, thanks @MartijnVisser I rebased the branch after conflicts with FLINK-31834 occurred and will go ahead and merge the change. |
What is the purpose of the change
Trying to integrate mirrors in case the azure mirror is not available. Original source is actions/runner-images#7048 (comment)
Brief change log
Creates
mirror.txtand integrate it into the/etc/apt/sources.listVerifying this change
CI run succeeds
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation