feat(grpc): allow files to be read by add_metadata_jmespath_expression

[tomgr]

The motivation for this change is that I would like to be able k8s bound service account tokens and gcloud service account id tokens into outgoing grpc requests. However, these tokens often have relatively short maximum validities and therefore I need a way to get buildbarn to reload these periodically, like it does for TLS certificates. This PR implements a general file-based mechanism for injecting file contents into outgoing gRPC requests. This seemed to me like a reasonably general way of doing it, but it's just an initial idea -- I'm open to doing this in a different way.

This allows add_metadata_jmespath_expression to refer to the contents of certain pre-declared files which are reloaded on a user-specified schedule. For example, if you want to use k8s bound service account tokens you could add the following to your Pod:

  volumes:
  - name: token-vol projected: sources:
      - serviceAccountToken: audience: my-buildbarn-instance expirationSeconds: 3600 path: buildbarn

Assuming this is mounted at '/tokens', you could then specify in your buildbarn config:

  addMetadataJmespathExpression: |||
    {
      "authorization": [join(' ', ['bearer', files.token])]
    }
  |||,
  addMetadataJmespathFiles: [
    {
      key: "token",
      path: "/tokens/buildbarn",
      refreshInterval: "1800s",
    }
  ]

This is quite useful for k8s service account tokens, as the maximum validity is often capped. Likewise this can also be used for Google service account id tokens, which also have a relatively short maximum validity.

[aspect-workflows]

Test

26 test targets passed

Targets

//pkg/auth:auth_test [k8-fastbuild]                                           897ms
//pkg/blobstore/buffer:buffer_test [k8-fastbuild]                             85ms
//pkg/blobstore/completenesschecking:completenesschecking_test [k8-fastbuild] 117ms
//pkg/blobstore/grpcclients:grpcclients_test [k8-fastbuild]                   122ms
//pkg/blobstore/grpcservers:grpcservers_test [k8-fastbuild]                   94ms
//pkg/blobstore/local:local_test [k8-fastbuild]                               227ms
//pkg/blobstore/mirrored:mirrored_test [k8-fastbuild]                         176ms
//pkg/blobstore/readcaching:readcaching_test [k8-fastbuild]                   48ms
//pkg/blobstore/readfallback:readfallback_test [k8-fastbuild]                 129ms
//pkg/blobstore/replication:replication_test [k8-fastbuild]                   103ms
//pkg/blobstore/sharding/integration:integration [k8-fastbuild]               113ms
//pkg/blobstore/sharding/legacy:legacy_test [k8-fastbuild]                    473ms
//pkg/blobstore/sharding:sharding_test [k8-fastbuild]                         998ms
//pkg/blobstore:blobstore_test [k8-fastbuild]                                 66ms
//pkg/builder:builder_test [k8-fastbuild]                                     68ms
//pkg/capabilities:capabilities_test [k8-fastbuild]                           80ms
//pkg/digest:digest_test [k8-fastbuild]                                       83ms
//pkg/filesystem/path:path_test [k8-fastbuild]                                77ms
//pkg/grpc:grpc_test [k8-fastbuild]                                           123ms
//pkg/http:http_test [k8-fastbuild]                                           155ms
//pkg/jmespath:jmespath_test [k8-fastbuild]                                   228ms
//pkg/jwt:jwt_test [k8-fastbuild]                                             91ms
//pkg/otel:otel_test [k8-fastbuild]                                           55ms
//pkg/prometheus:prometheus_test [k8-fastbuild]                               125ms
//pkg/util:util_test [k8-fastbuild]                                           60ms
//pkg/x509:x509_test [k8-fastbuild]                                           91ms

Total test execution time was 5s. 5 tests (16.1%) were fully cached saving 591ms.

[EdSchouten]

Hey Thomas,

I'd be open to having a feature like this, but I do think that if we add it, it shouldn't be restricted to just this single place. What makes this place so special that we may accept files, while others cannot? There are also some features related to JMESPath that we should eventually consider adding. For example, it would be nice if any JMESPath expression in config may be accompanied with test vectors that are validated during startup.

What are your thoughts on adding a pkg/proto/jmespath with:

message File {
  ...
}

// No need to add it now, but would be logical to provide eventually:
message TestVector {
  google.protobuf.Value input = 1;
  google.protobuf.Value expected_output = 2;
}

message Expression {
  string expression = 1;
  repeated File files = 2; 
  repeated TestVector test_vectors = 3;
}

What are your thoughts on this?

[tomgr]

Hi Ed,

That's true - there's nothing special about this. I like your idea. Are you happy for me to migrate all of them? I think this might be the existing list:

./pkg/proto/configuration/jwt/jwt.proto:69:  string claims_validation_jmespath_expression = 5;
./pkg/proto/configuration/jwt/jwt.proto:90:  string metadata_extraction_jmespath_expression = 6;
./pkg/proto/configuration/grpc/grpc.proto:90:  string add_metadata_jmespath_expression = 12;
./pkg/proto/configuration/grpc/grpc.proto:368:    string peer_credentials_jmespath_expression = 6;
./pkg/proto/configuration/grpc/grpc.proto:435:  string validation_jmespath_expression = 3;
./pkg/proto/configuration/grpc/grpc.proto:448:  string metadata_extraction_jmespath_expression = 4;
./pkg/proto/configuration/auth/auth.proto:41:    string jmespath_expression = 4;
./pkg/proto/configuration/http/http.proto:149:  string metadata_extraction_jmespath_expression = 6;

I presume the intent is that all of those will change type to be buildbarn.jmespath.Expression?

[EdSchouten]

Exactly! Feel free to change all of them.

[tomgr]

I've gone ahead and made this change, and I've also added the test vectors whilst I was at it. I'd find that useful.

The code now optionally takes a program.Group. This is wired into a few places, but not into the overall grpc Client construction as that's a larger refactor.

One thing to note here is that the Search function that I have exposed takes a map[string]any instead of a any (unlike the go-jmespath library). This was fine in every place in the codebase except for the OIDC claims processing in pkg/http/oidc_authenticator.go where I had to change the types that the claims were unmarshled to. However my understanding is that oidc claims are guaranteed to be JSON dictionaries, so I don't think this is an issue.

[tomgr]

Thanks for the review - I've marked the conversations as resolved when I've done the corresponding thing. If you'd prefer a different workflow please let me know.

[EdSchouten]

Thanks for continuing to work on this, @tomgr!

[EdSchouten]

Hey Thomas,

Thanks again for working on this. I have one small comment, and once that's dealt with we can merge this.