Repository navigation
feat(compute-envs): support Seqera Compute allowed buckets [COMP-2620] - #729
Merged
Merged
Conversation
API 1.250.0 is the version staged for the next prod deploy and carries the Seqera Compute bring-your-own-bucket fields. The native image only serializes the accessors listed in reachability-metadata.json, so the new allowBuckets and bucketPolicies accessors are registered for the models tw reads or writes them through. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Seqera Compute environments can now read and write customer S3 buckets besides the managed work directory bucket. The list is set at creation only, so it belongs on 'add'; Platform validates the format and the 10-entry limit and returns the error as is. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An allowed bucket lives in the customer's AWS account, so access also needs a bucket policy trusting the compute environment's roles. Platform renders one per bucket once the roles exist; the text view now prints them ready to copy, as the UI does. The JSON output already carries them in bucketPolicies. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approve. This is an additive change: a new optional --allow-buckets flag on compute-envs add seqera-compute, a "Bucket policies" section in compute-envs view, and the SDK bump plus reachability entries that both of these need.
What I checked:
SeqeraComputePlatform: if--allow-bucketsis left out,allowBucketsstaysnull. The existingtestAddbody still matches, and mockserver's JSON matching is lenient, so requests without the flag are unchanged.split = ","matches how other list options in the CLI are parsed.ComputeEnvView.printBucketPolicies: it returns early when the map is null or empty, so other platforms and environments still being created print nothing new. The JSON output (getJSON()returns the DTO) already includesbucketPolicies, and the JSON test confirms this.conf/reachability-metadata.json: the diff only adds entries, and they cover the new getters and setters onComputeEnvResponseDtoand on both Seqera Compute config classes.
Non-blocking notes:
- The PR depends on
tower-java-sdk:1.250.0(seqeralabs/tower-sdk-gencode#133). CI can't resolve it until that is published, so I could not see a green build here. The native image build isn't covered either, as the description says. Please confirm both before merging. policy.replaceAll(...)assumes every value in the map is non-null. That should hold for API output. AString.valueOf(policy)or a null skip would cost nothing if you want to be defensive.- The
--allow-bucketshelp text says the value can't be changed after creation. That's accurate, since the option only exists onadd.
I did not run the tests or build anything locally. This review is based on reading the diff and the code around it.
1.251.0 declares secretsKmsKeyId on the AWS Batch and AWS Cloud configs as nullable, so the SDK turns it into a JsonNullable and calling its setter with null now sends an explicit null instead of leaving it out. tw only sets it when --secrets-kms-key is given, as it already does for the other JsonNullable fields, and the new accessors are registered for the native image. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The native binary tests failed with MissingReflectionRegistrationError: models listed method by method in reachability-metadata.json lacked members added between SDK 1.243.0 and 1.251.0, such as the globalCache* accessors, DataStudioDto.allowedToConnect and the read-only constructor ComputeEnvResponseDto(Map). Every public member of the published 1.251.0 jar missing from those entries is now listed; nothing is removed. The bucket policy view test also expected '\n' while the view prints '%n', which is '\r\n' on Windows. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
stefanoboriero
requested changes
Oct 27, 2026
stefanoboriero
left a comment
stefanoboriero
left a comment
Contributor
There was a problem hiding this comment.
Remember to update the minimum API version required in https://github.com/seqeralabs/tower-cli/blob/master/VERSION-API. The rest of the changes look good 👍
--allow-buckets needs SeqeraComputeConfig.allowBuckets (API 1.246.0) and compute-envs view reads bucketPolicies (API 1.247.0), so tw info should warn against older Platforms. The 1.251.0 SDK changes are client-side only, and prod already runs 1.250.0. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
stefanoboriero
approved these changes
Oct 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes COMP-2620
What
tower-java-sdkfrom 1.243.0 (in two steps, 1.250.0 then 1.251.0). Every public member of the 1.251.0 models thatconf/reachability-metadata.jsonlists method by method is now registered (only additions). That covers the BYOB fields plus members added since 1.243.0, such asglobalCache*andallowedToConnect, without which the native binary failed on those models.secretsKmsKeyIdbecameJsonNullablein 1.251.0 (AWS Batch and AWS Cloud): its setter would now send an explicitnull. tw only sets it when--secrets-kms-keyis given, as it already does for the otherJsonNullablefields. The new tests check that the key is left out when the option isn't given.tw compute-envs add seqera-compute --allow-buckets s3://bucket,s3://bucket/path: sets the customer S3 buckets a Seqera Compute environment can read and write. It can only be set at creation. Platform validates the format and the 10-entry limit, and its error is shown as is.tw compute-envs view: for Seqera Compute, prints a "Bucket policies" section with one copy-ready policy per bucket, as the UI does. Nothing is printed until Platform has created the roles. The JSON output already carriedbucketPolicies.compute-envs list -o jsonand the studio JSON output pick up the new read-onlyallowBucketswithout changes. Studio mounts need nothing either: tw doesn't check data-link compatibility itself (only the UI does).Depends on
seqeralabs/tower-sdk-gencode#134, which fixes the failed publish of
tower-java-sdk:1.251.0. After merging it,publish.ymlneeds a manual run. CI here can't resolve the SDK until then.How to verify
./gradlew test: 909 tests, 0 failures, against SDK 1.251.0 installed withpublishToMavenLocalsecretsKmsKeyIdguards removed, bothtestAddWithoutSecretsKmsKeyOmitsIttests failallowBucketswiring removed,testAddWithAllowBucketsfailsviewtext outputtw compute-envs add seqera-compute -n byob -r eu-west-1 --allow-buckets s3://<bucket>, thentw compute-envs view -n byobonce AVAILABLE shows the policy🤖 Generated with Claude Code