release: dynamic connector VERSION file

[mdibaiee]

Description:

• Adds a VERSION file to each connector that specifies the version of the connector (at the moment it's v1 for all)
• This allows us to bump a version when necessary

Workflow steps:

• Run CI

Documentation links affected:

N/A

Notes for reviewers:

(anything that might help someone review this PR)

---

This change is 

[psFried]

I think this seems directionally right, but had a few comments.

[psFried]

I'm not seeing why we'd want the version applied to the base image. All of the connectors Dockerfiles that use the base image hard-code the version as :v1. But that's also not making a ton of sense to me, since the build always builds and tags the base image with :local, and only tags it with :v1 on a push to main. So I'd think as it is, that if you make a change to the base image in a PR, then the connector builds won't actually use that change until after it's been merged to main.

Basically, this seems like it was already broken before this PR, but adding the dynamic version to base image definitely doesn't seem like it would help. I'm not confident that I fully understand the purpose of base-image, so take this with a grain of salt. But it seems to me like we could only build the :local tag of the base image and just use that in the connector builds instead of v1.

[mdibaiee]

@psFried

So I'd think as it is, that if you make a change to the base image in a PR, then the connector builds won't actually use that change until after it's been merged to main.

That's true, I don't know what's the best way out of that. If you really want to try something in your PR, you can change your PR to include a change to a connector to use base-image:local or something of the sort.

But it seems to me like we could only build the :local tag of the base image and just use that in the connector builds instead of v1.

The idea is we want the base-image to be available to 3rd-parties who want to create connectors for Flow as well, because we have certain requirements that might change (think: we added OpenSSH requirements to docker images, base image serves that).

[mdibaiee]

I don't foresee breaking changes in the base-image to be frequent at all. We will realistically only want to break the base-image if we are making some serious changes to how we run connectors.

[willdonnelly]

My two cents: I think given that we want base-image to be published to ghcr.io for others to use it does make sense to give it a version tag, but it seems confusing and unexpected that when making a change to base-image in a PR the new connectors built by that CI run won't include the change. Could we use the :local tag in our connector builds but still push the version-file-tagged base-image?

[willdonnelly]

It looks like CI builds are failing because the ghcr.io/estuary/base-image:local image built in one job isn't visible to the other matrix job. Cursory googling suggests that you can export the whole Docker image as a file and move that between jobs though (see docker/build-push-action#225 (comment) for instance).

[mdibaiee]

I tried my best on this, but no luck.

Apparently docker buildx does not pick up local images: docker/buildx#847
This means even if we move the image from the build-base to build-connector job, it doesn't get picked up by buildx. It doesn't even get picked up if you build the base-image in build-connector job, it always tries to fetch the image from remote, and will fail if it doesn't exist.

There is probably one hacky way around this which is spinning up a local registry in the CI job, pushing to that local registry and using that, but that means our Dockerfiles will have FROM localhost:5000/base-image:local in them, which makes building them on our local machines cumbersome and hacky and unintuitive.

All in all, I don't really like this solution. I'd say let's try to live with this until buildx supports a way of using local images in its builds.

[willdonnelly]

Yeah, I think we've at least established that this is complex enough it shouldn't be tacked on to this change.

(That said, I think it may still be possible to make local builds work via something like ARG BASE_IMAGE=ghcr.io/estuary/base-image:local and then FROM ${BASE_IMAGE} (relevant docs), and then by overriding the BASE_IMAGE variable during our CI builds it could be pointed at another registry)

[mdibaiee]

@willdonnelly that's a good idea, given that it seems to support value substitution just like bash (e.g. ${variable:-default}), we can default to v1 image to allow local builds to run just fine. In this case, we don't even need to move the image, or a local registry, since we do publish the hash-tagged base-image. We can just point to that. I'll look at it tomorrow.

[mdibaiee]

Got it working 👍🏽

[mdibaiee]

So now image builds in CI (except the final dev + version build of connectors) use the local base-image

[psFried]

This looks great! Thanks for also fixing the base image handling. The introduction of the changelogs also seems like a good idea.

[willdonnelly]

LGTM, thanks for getting this working!