impl(auth): accept trust boundary header to build auth headers

[alvarowolfx]

Towards #4186

[codecov]

Codecov Report

❌ Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.00%. Comparing base (c20ee3a) to head (a8cd689).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/auth/src/headers_util.rs	94.87%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4528      +/-   ##
==========================================
- Coverage   95.03%   95.00%   -0.04%     
==========================================
  Files         195      195              
  Lines        7428     7440      +12     
==========================================
+ Hits         7059     7068       +9     
- Misses        369      372       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coryan]

More questions and suggestions than hard requirements. Maybe explain why the constant needs to go in a different place.

[alvarowolfx]

@coryan I started with the suggestion to accept a header map, but them each consumer of the build_cacheable_header would have to also know the header name, which I felt like it was going to make is less useful. I added a AuthHeader struct that will be updated as we add support for new headers and it's easier to consumers to support new fields add to it, but still using the build_cacheable_header helper. LMK what you think

[alvarowolfx]

One thing that I'm still not particularly happy is that there are more clones happening now, I'm seeing how to avoid that.

[alvarowolfx]

@coryan I moved to use Option<&'a str> and added the lifetime 'a to other parts of the code to avoid clones. I think there are two options here, which I'm not sure which one is better, but both works:

1. Have a new(&token) and a build() method that return the CacheableResource (the one that I pushed here). I need the AuthHeadersBuilder to be it's own struct instead of just extending HeaderMap, as I need to know if is a Bearer token or API Key and also hold the &token.
2. Have a new() , a build_bearer(&token) and build_api_key(&token), so AuthHeadersBuilder can just extend HeaderMap and don't need to hold &token and header type.

[coryan]

@coryan I moved to use Option<&'a str> and added the lifetime 'a to other parts of the code to avoid clones.

I think the code looks good enough to merge and then iterate. I still think we could improve it.

I think there are two options here, which I'm not sure which one is better, but both works:

1. Have a new(&token) and a build() method that return the CacheableResource (the one that I pushed here). I need the AuthHeadersBuilder to be it's own struct instead of just extending HeaderMap, as I need to know if is a Bearer token or API Key and also hold the &token.

Couldn't you create the header map in new() or for_api_key(), but only if the token is a CacheableResource::New? That is, do the cacheable thing early instead of when calling .build()?

[coryan]

As an example, this code could read:

match &token {
    CacheableResource::NotModified => Ok(CacheableResource::NotModified),
    CacheableResource::New { entity_tag, data } => {
        let headers = HeaderMapBuilder::from_bearer_token(data)?
            .maybe_quota_project_id(self.quota_project_id.as_deref())?
            .build();
        Ok(CacheableResource::New { entity_tag.clone(), headers })
    },
}

and if we want to say "monad", we could implement .map() and transpose() functions for CacheableResource so we can say:

    token.map(|data| -> Result<HeaderMap> {
            let headers = HeaderMapBuilder::from_bearer_token(data)?
                .maybe_quota_project_id(self.quota_project_id.as_deref())?
                .build();
            Ok(headers)
        })
        .transpose() // `CacheableResource<Result<T>>` to `Result<CacheableResource<T>>`
    },
}

[coryan]

Approved to unblock you, but as I said, this could be better.