Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow origins to export multiple prefixes #927

Merged
merged 19 commits into from
Mar 13, 2024

Conversation

jhiemstrawisc
Copy link
Member

@jhiemstrawisc jhiemstrawisc commented Mar 11, 2024

This should be the first step toward multi-export origins. While I didn't get
things sorted out for S3, the fact is the S3 plugin doesn't really support that
yet anyway. For now, this appears to be working with POSIX, which is the majority
of origins anyway.

In particular, the larges addition as I see it is the GetOriginExports function,
which allows us to construct a list of OriginExports that can be used in a standard
way elsewhere.

Closees #858

@jhiemstrawisc
Copy link
Member Author

@CannonLock Just wanted to ping you to make sure you see the new addition here from over the weekend. We now have the object:

Origin:
  Exports:
    - StoragePrefix: "/foo"
       FederationPrefix: "/bar"
       Capabilities: ["PublicReads", "DirectReads", "Writes", "Listings"]
    - <Another export>

Copy link
Collaborator

@turetske turetske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preliminary review - still need to dig deep into some of the code.

docs/parameters.yaml Outdated Show resolved Hide resolved
common/director.go Show resolved Hide resolved
director/advertise.go Outdated Show resolved Hide resolved
generate/param_generator.go Outdated Show resolved Hide resolved
generate/param_generator.go Outdated Show resolved Hide resolved
xrootd/authorization_test.go Show resolved Hide resolved
xrootd/authorization_test.go Show resolved Hide resolved
common/director.go Show resolved Hide resolved
client/handle_http_test.go Outdated Show resolved Hide resolved
server_ui/advertise.go Show resolved Hide resolved
@turetske
Copy link
Collaborator

Other files that also contain environment params to fix: github_scripts/stat_test.sh

@CannonLock
Copy link
Contributor

@jhiemstrawisc Is this the full set of values that Capabilities can be?

Capabilities: ["PublicReads", "DirectReads", "Writes", "Listings"]

@jhiemstrawisc
Copy link
Member Author

@jhiemstrawisc Is this the full set of values that Capabilities can be?

Capabilities: ["PublicReads", "DirectReads", "Writes", "Listings"]

Good catch, I missed one there. There's also "Reads", which turns on authenticated reads. Setting "PublicReads" implies "Reads" (you can't read publicly if you don't allow any reading at all), but a capabilities flow without either "Reads" or "PublicReads" implies that it's a write-only origin.

@jhiemstrawisc jhiemstrawisc requested a review from turetske March 11, 2024 18:48
@CannonLock
Copy link
Contributor

@jhiemstrawisc can multiple values be true at the same time or can you only pick one?

@turetske
Copy link
Collaborator

@jhiemstrawisc can multiple values be true at the same time or can you only pick one?

Multiple values. You can have Writes and PublicReads both be true, for example.

turetske
turetske previously approved these changes Mar 11, 2024
Copy link
Collaborator

@turetske turetske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me, but obviously needs a rebase for the merge conflicts and hopefully Cannon's fixes in there, soon.

@turetske turetske self-requested a review March 11, 2024 21:15
This should be the first step toward multi-export origins. While I didn't get
things sorted out for S3, the fact is the S3 plugin doesn't really support that
yet anyway. For now, this appears to be working with POSIX, which is the majority
of origins anyway.

In particular, the larges addition as I see it is the `GetOriginExports` function,
which allows us to construct a list of OriginExports that can be used in a standard
way elsewhere.
@turetske turetske dismissed their stale review March 11, 2024 21:40

Trying to force tests again

@turetske turetske closed this Mar 12, 2024
@turetske turetske reopened this Mar 12, 2024
@turetske turetske force-pushed the issue-858 branch 2 times, most recently from 9152efc to fcc923f Compare March 13, 2024 17:05
@turetske turetske merged commit cdff9a6 into PelicanPlatform:main Mar 13, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants