-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat(controlplane): CAS download endpoint #307
feat(controlplane): CAS download endpoint #307
Conversation
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
9ff9c7e
to
47f6aad
Compare
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
app/cli/cmd/auth_login.go
Outdated
@@ -71,6 +71,7 @@ func interactiveAuth() error { | |||
// Append local callback URL | |||
q := serverLoginURL.Query() | |||
q.Set("callback", callbackURL.String()) | |||
q.Set("long-lived", "true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This change will require users to update their CLIs otherwise their tokens will expire constantly.
I decided to go this way because
- It just affects operators CLI, not the attestation CLI
- Doing the opposite, meaning making long-lived default while short-lived optional is not good practice.
- We are still pre-1.0 so we do not guarantee backwards compatibility.
- I do not think we can detect the version of the CLI yet, so there is no way to show a warning message to the user.
Please let me know if you have any objection on this breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please elaborate on "It just affects operators CLI, not the attestation CLI"?
You mean that it does not affect RobotAccount auth based calls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -5,7 +5,7 @@ | |||
server: | |||
http: | |||
addr: 0.0.0.0:8000 | |||
timeout: 1s | |||
timeout: 10s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 10 seconds? Just to be on the safe side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need now a longer timeout since the HTTP endpoint handler now will check if the cas backend is valid in some cases, that takes some time.
We have currently configured 10 seconds in the similar gRPC endpoints so I thought of using the same.
app/cli/cmd/auth_login.go
Outdated
@@ -71,6 +71,7 @@ func interactiveAuth() error { | |||
// Append local callback URL | |||
q := serverLoginURL.Query() | |||
q.Set("callback", callbackURL.String()) | |||
q.Set("long-lived", "true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please elaborate on "It just affects operators CLI, not the attestation CLI"?
You mean that it does not affect RobotAccount auth based calls?
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Enables two endpoints in the controlplane APIs
GetDownloadURL (gRPC)
and/download (http)
GetDownloadURL (gRPC)
The first endpoint will be used by any client to retrieve the CAS download URL, this could be useful for clients as UIs, where a downloadURL could be generated on-demand after an user action.
The returned link includes a temporary download token, valid for only 10 seconds.
/download/sha256:[hex]
(http)This endpoint can be used to share links to download items in the CAS. By accessing the link the handler will
GetDownloadURL
You can see it in action in both a browser below
download.mp4
Refs #293