feat!: add spanner rest port and upgrade google-cloud-sdk version

[plaflamme]

This does 2 things:

• exposes Spanner's REST API port
• update the google-cloud-sdk image to the latest version and its latest hosting registry (https://cloud.google.com/sdk/docs/downloads-docker)

[DDtKey]

Thank you for the contribution! 🙏

I've left couple of comments, please take a look

[DDtKey]

That's definitely a breaking change: publicly visible variable was changed

We can consider a breaking change if we want to update the image version as well.
But if we won't change anything else, doesn't make a lot of sense. We could keep SPANNER_PORT as (deprecated) variable = SPANNER_GRPC_PORT until the next major release

[plaflamme]

I can leave SPANNER_PORT unchanged (or marked as deprecated), but a breaking change is unavoidable since we also need to change pub struct CloudSdkArgs here. Presumably, this is less likely to break people, but it's still a breaking API change.

Do we care about that other breaking change?

[DDtKey]

Well, in that case I think we can go only with breaking change flag.
Thus, I think it's ok to bump the version as well. But better to preserve the registry as dockerhub

I'd suggest to refactor CloudSdkArgs a bit to avoid such breaking changes in the future, e.g: make the fields private and expose builder-like functions. So we can add new arguments in the future without breaking changes, just by adding .with_my_arg(...) (just for example)

WDYT?

[plaflamme]

Yeah, that makes sense. I'll take a stab at this later today.

[DDtKey]

Hi there!

Any progress? We're going to prepare a new release with breaking changes soon.
So would be nice to incorporate this one as well

[plaflamme]

I took a stab at this, but it was a larger endeavour than I expected. After digging into the details, I realized that the current abstractions are more or less "in the way".

The reality is that the gcloud command line is structured like so:

gcloud <global args> emulators <emulator> start <emulator args>

where:

• global args applies to all emulators (because it actually applies to the gcloud command)
• emulator is one of spanner | bigtable | ..., but may require gcloud beta emulators or not (depends on the version of the image)
• emulator args are specific to the emulator, but also happen to overlap (e.g.: --host-port host:port is present on all of them, but it's not like it's guaranteed to be the case)

The current structure "mixes" things, for example the project field on Datastore is actually part of global args and should be available on all emulators, not just this one.

Lastly, after comparing with other similar containers, like cockroachdb, elastic-search and consul, they all just take arbitrary lists of arguments without trying to formalize them into types.

So I was faced with 2 options:

1. do a bunch of work to "correctly" formalize the arguments
2. delete most code and just take, maybe, 2 Vec<String>s, one for gcloud and one for the emulator

Option 2 seems better IMO because it will be more amenable to using different versions of the sdk (which may have different command line arguments then the ones formalized in this library). So it would ultimately be less work to maintain, but makes it harder for users to use...

Both options are actually a non-trivial amount of work (2 is probably less, but still). Maybe an approach would be to make this a new submodule? e.g.: gcloud_sdk which would map to the cloud-sdk image, but not assume you want to run emulators. There are actually other reasons why you may want to use this image as-is without using the emulators, for example, you can use that image to do stuff against "actual" GCP, like create test instances of databases, or GCS buckets etc.

Any opinions?

[DDtKey]

In general, we could go with 2 Vec<String>, but accept something like &[impl Into<String>] (but provide methods with meaningful names). This way, users can define their own structures which implement Display (e.g enums).

For test scenarios it's usually ok to hardcode some values or create simple wrappers.
Otherwise it can be too complicated to support all possible arguments. It could lead to a mess (especially across different versions)

For example, ENV variables also could be represented as typed structs, but it's too complicated for testing purposes. And additional complexity for maintainers.

IMO, providing strong-typed interface makes sense only for some required and limited parameters. Emulator looks like a good candidate, even though it's not really required

---

Maybe an approach would be to make this a new submodule?

In general this sounds reasonable, but it's not entirely accurate because these reasons don't look like the case we want to support as is:

you can use that image to do stuff against "actual" GCP, like create test instances of databases, or GCS buckets etc.

These cases just don't seem like the "test containers" use case.