fix(gcs): do not skip signing with allow_anonymous

[jdockerty]

Which issue does this PR close?

Fixes an oversight I made in #4965

Rationale for this change

allow_anonymous is for allowing access when explicitly allowed and credentials are not given (e.g. public buckets) and not for skipping signing entirely.

[Xuanwo]

Thanks so much for the quick fix. Could you also help with load_token? I think the logic should be to skip signing if token.is_none(). Additionally, load_token should correctly manage allow_anonymous.

[jdockerty]

skip signing if token.is_none()

I believe this is for the other PR which introduces the token: Option<String> field?

[Xuanwo]

I believe this is for the other PR which introduces the token: Option<String> field?

I mean:

• load_token() should return Result<Option<GoogleToken>>
• if self.token is Some(), we should return it first load_token. (this can be done in the other PR)
• If load_token() returns None, sign should skip it.

[jdockerty]

Ah yeah, I believe we're talking about the same things here.

I pushed 54c1ae8 just as you wrote this. I believe this covers what you mention here.

[Xuanwo]

Great, thanks!

[Xuanwo]

Looks good to me now, thanks a lot!

[Xuanwo]

This is the last PR I gonna to merge today. 00::39 AM now ❤️

I will review your other PR tomorrow.

[jdockerty]

No problem! Get some sleep 🚀

[jdockerty]

@Xuanwo If you don't mind, as a small follow-up on this one.

Do I understand rightly that I need slightly alter how the load_token function works here with allow_anonymous.

Reading the Java implementation from Iceberg's FileIO, the GCS_NO_AUTH here is explicitly set for testing purposes and allows no credentials.

I think the equivalent of that here is that the load_token function returns instantly when GCS_NO_AUTH is set, or am I misunderstanding this?

impl GcsCore {
    async fn load_token(&self) -> Result<Option<GoogleToken>> {
        if self.allow_anonymous {
          return Ok(None);
        }
        // proceed to load...

I ask because the tests I have for the iceberg-rust PR with GCS_NO_AUTH no longer work after using latest changes on main (which includes this PR), but I think it is because I've muddled up the meaning of GCS_NO_AUTH and anonymous access.

[Xuanwo]

Hi @jdockerty, I believe GCS_NO_AUTH should be equivalent to allow_anonymous + disable_vm_metadata + disable_config_load.

[jdockerty]

Ah! Thank you, that is the piece I was missing 👍