cirruslabs / cirrus-ci-agent

Agent to execute Cirrus CI tasks
Mozilla Public License 2.0
13 stars 6 forks source link

Support artifacts upload over HTTPS #263

Closed edigaryev closed 1 year ago

edigaryev commented 1 year ago

See https://github.com/cirruslabs/cirrus-ci/issues/308.

edigaryev commented 1 year ago

Do we need the old gRPC uploader? It's the backed problem to support both of the options for some time until we fully migrate to the new agent version.

I think it's worth keeping it for now, not all storage backends support uploading via HTTPS, and it would be nice to have a fallback if HTTPS artifact upload goes wrong, as it's relatively new code.

fkorotkov commented 1 year ago

We already only support backends that can generate URLs. cache instruction is only working via generation of pre-signed URLs.

Our internal integration tests pretty decently cover caching and artifacts logic. Should be pretty safe to deploy if this chancge will pass them.

edigaryev commented 1 year ago

cache instruction is only working via generation of pre-signed URLs.

But the agent still supports gRPC fallback even for cache uploads:

https://github.com/cirruslabs/cirrus-ci-agent/blob/97f19e6376b39def3f33f17f7f9b233aeee62370/internal/http_cache/http_cache.go#L197-L203

There's also a Cirrus CLI, which uses gRPC to accept cache and artifacts.

edigaryev commented 1 year ago

LGTM overall. Have you tried to build a docker container locally and run integration tests in cirrus-ci repo to check they work with the new version? cirrus-ci repo uses S3 emulator and can presign url for local use so might be a good idea to try before merging.

The artifacts-related integration tests seem to run just fine for me.