feat(subscription): support oidcToken

[stayradiated]

Issue

It is currently not possible to create a new Subscription with the Service Account Email or Audience properties configured. These properties are part of the PushConfig.oidcToken field which is not being formatted correctly.

Changes

Similar to the pushEndpoint option, when formatting the metadata of a Subscription, the property needs to be moved into the pushConfig object.

    if (metadata.pushEndpoint) {
      formatted.pushConfig = {
        pushEndpoint: metadata.pushEndpoint,
      };
      delete formatted.pushEndpoint;
    }

    // fix
    if (metadata.oidcToken) {
      formatted.pushConfig = {
        ...formatted.pushConfig,
        oidcToken: metadata.oidcToken,
      };
      delete formatted.oidcToken;
    }

I have added tests and updated documentation for PubSub.createSubscription and Subscription.modifyPushConfig.

[feywind]

@bcoe Code review wise this looks fine to me, but I don't know all the inner workings enough to give a yay/nay on the functionality. Do you mind taking a quick look tomorrow? (Or tag someone who would know...)

[codecov]

Codecov Report

Merging #865 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #865   +/-   ##
======================================
  Coverage    83.5%   83.5%           
======================================
  Files          31      31           
  Lines       11269   11269           
  Branches      333     333           
======================================
  Hits         9410    9410           
  Misses       1858    1858           
  Partials        1       1

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d159f71...e089ff7. Read the comment docs.

[codecov]

Codecov Report

Merging #865 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #865      +/-   ##
==========================================
+ Coverage   83.46%   83.50%   +0.03%     
==========================================
  Files          31       31              
  Lines       11246    11269      +23     
  Branches      332      333       +1     
==========================================
+ Hits         9387     9410      +23     
  Misses       1858     1858              
  Partials        1        1              

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32ae411...d159f71. Read the comment docs.

[bcoe]

@feywind this looks reasonable to me too, but I think perhaps we should loop in @bshaffer for a review who's been doing work around ID Tokens.

[bcoe]

@stayradiated have you tested end to end with your branch against the actual service?

[bcoe]

I'm relatively green in this codebase, but mind clarifying why we need to delete the key off the formatted object?

[bshaffer]

It does seem to already be happening above it for formatted.pushEndpoint and as well...

[feywind]

Yeah, that was my take as well, oidcToken is just being treated like what's above. My main question is on the sort of systemic goodness of the patch - I don't understand the gestalt of the library enough to say yay or nay yet.

[bshaffer]

I believe the changes here just expose access to the underlying API call, and is already exposed in other client libraries. With the exception of deleting the key (is this for memory management?) this is LGTM.