Design: Velero client download APIServer #7344

kaovilai · 2024-01-23T02:11:31Z

Signed-off-by: Tiger Kaovilai tkaovila@redhat.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #7432

Please indicate you've done the following:

Accepted the DCO. Commits without the DCO will delay acceptance.
Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
Updated the corresponding documentation in site/content/docs/main.

Slack threads:

examples of immutable resources https://kubernetes.slack.com/archives/C021GPR1L3S/p1709329227490779

codecov · 2024-01-23T02:19:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.83%. Comparing base (bc29471) to head (119ac46).
Report is 142 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7344      +/-   ##
==========================================
+ Coverage   58.49%   58.83%   +0.34%     
==========================================
  Files         343      345       +2     
  Lines       28457    28798     +341     
==========================================
+ Hits        16646    16944     +298     
- Misses      10392    10425      +33     
- Partials     1419     1429      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

design/velero-client-download-apiserver_design.md

kaovilai · 2024-01-31T20:24:08Z

/kind changelog-not-required

design/velero-client-download-apiserver_design.md

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> CRD changes Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> Add service loadbalancer approach Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> Added api aggregation downsides and notes around ingress in velero helm chart Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> Add Comparison table of approaches Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

design/velero-client-download-apiserver_design.md

ywk253100 · 2024-04-17T10:39:49Z

design/velero-client-download-apiserver_design.md

+
+This approach may also [work on KinD clusters](https://kind.sigs.k8s.io/docs/user/loadbalancer/)
+
+The current DownloadRequest CR status `veleroServerURL` field will be populated to use the Download Server URL instead of the signed object store URL where available.


The alternative approach is that the DownloadRequest Reconciler decides the correct way to download the data and return the URL, the Velero CLI doesn't need to care about the details, it just gets the URL from the DownloadURL field.
The benefit of this approach is that there are some other downstream products/projects rely on the DownloadRequest directly to download the data, they don't need to change to work with the download server.

IIUC @ywk253100, you are advocating that for some object stores, we would want to use the signed URL that is returned and not the download request when it is not needed?

If that is the case I agree, I wonder how we will determine that though?

IIUC @ywk253100, you are advocating that for some object stores, we would want to use the signed URL that is returned and not the download request when it is not needed?

No, What I'm saying is that we always read the URL from the downloadURL field of the download request rather than adding a new veleroServerURL.

If that is the case I agree, I wonder how we will determine that though?

We can add a new field in the BSL spec to determine how to get the URL.

I like this idea.

decides the correct way to download the data

Not sure if there's an easy way to determine from DR reconciler what cli has access to.. CLI can perhaps create another CR saying prior attempt was unsuccessful so the next DR url returned would be download server url.

If we add a new field (e.g. downloadViaVeleroServer bool) to the BSL spec, the DR reconciler can determine which URL (the URL of direct access or via the Velero server) should be returned.
And the CLI doesn't need to care about this, it downloads the data from the downloadURL of DR.

ywk253100 · 2024-04-17T10:42:21Z

design/velero-client-download-apiserver_design.md

+
+The Velero CLI will still default to `downloadURL` from DownloadRequest but can fallback or forced to `veleroServerURL` which is served via the Velero server.
+
+If velero CLI attempted request from veleroServerURL at the specified FQDN and failed, and velero CLI command was executed inside velero server pod (`alias velero='kubectl exec -it deployment/velero -- velero'`), velero CLI will attempt to use the service name (cluster.local address) to connect to the download server.


What is the command used here for?

This is for the case where user shell only has kubectl, and not velero CLI. They can access velero CLI via kubectl exec command.

If executing velero CLI from velero pod, it would be possible to use cluster.local address and/or localhost address.

I am a little confused about when this is used. Is this just for documentation for a user to be able to access things if they don't have a velero CLI installed?

A little technique, that could have code dedicated to it. Since download api server is a fallback, we can move this out of scope or remove entirely.

An example of where we told folks they could alias velero command so they don't need to actually install anything to their machine to use velero CLI functions.

https://access.redhat.com/articles/5456281#:~:text=alias%20velero%3D%27oc%20%2Dn%20openshift%2Dadp%20exec%20deployment/velero%20%2Dc%20velero%20%2Dit%20%2D%2D%20./velero%27

ywk253100 · 2024-04-18T09:25:34Z

design/velero-client-download-apiserver_design.md

+
+The Velero CLI may use the VeleroServerURL to download data from the Velero server when DownloadURL is not populated, unreachable, or explicitly requested via `--veleroServerUrl`.
+
+When using VeleroServerURL, Velero CLI will send current kubeconfig to the download server via HTTP Authorization Header for velero server to authenticate and authorize the request.


Velero CLI isn't the only client talks to the server, so it's possible there is no kubeconfig file.

How about we follow the similar way the object storage uses that generate a signed URL with a secret? Only the requests with the signed URL are valid.

Velero CLI isn't the only client talks to the server

I was not aware. Can you clarify what else talks to the server in this way?

In our downstream product, the client runs inside the pod and uses CRs directly to talk to the server.

Currently, if users have permission to create the DownloadReqeust then they have permission to download data from the object storage, and a signed URL is returned. The permission checking happens actually against the DownloadRequest. The object storage only checks the request is signed and valid. So we can follow the same pattern that in the download server side we only check the request is signed and valid.

Ok so just as long as DownloadRequests is able to get created, and the velero CLI have permission to read its status, then the signedUrl there should be usable without the kubeconfig mechanism proposed.

This is scary to have URLs that contain the entire backup that could be world accessible.

If we go down this path I think you have to have the link expire at some reasonable time

If we go down this path I think you have to have the link expire at some reasonable time

Yes, we can follow the way how object storage does

ywk253100 · 2024-04-18T09:29:18Z

design/velero-client-download-apiserver_design.md

+}
+```
+### HTTPS Server Certificates
+Velero server will generate a self-signed certificate for HTTPS server endpoint. The certificate will be stored in a secret in the same namespace as the Velero server. The secret will be named `velero-download-server-certs`.


This should be configurable

I would also explore when an ingress/router can use a CA cert and doesn't want to re-encrypt to talk to the service.

ywk253100 · 2024-04-18T10:08:36Z

design/velero-client-download-apiserver_design.md

+
+[![Velero Download Server](VeleroDownloadServer.jpg)](VeleroDownloadServer.jpg)
+
+## Detailed Design


Could you add a section about what the endpoint looks like?

Yes. Will do. I think we can simply use the same path pattern as to how the files are stored today.. so something like /backups/<backupName>/<contents>/...

shawn-hurley

I think this is looking really good! I am glad that we are making progress on this!

design/velero-client-download-apiserver_design.md

shawn-hurley · 2024-04-18T18:50:39Z

design/velero-client-download-apiserver_design.md

+
+This approach may also [work on KinD clusters](https://kind.sigs.k8s.io/docs/user/loadbalancer/)
+
+The current DownloadRequest CR status `veleroServerURL` field will be populated to use the Download Server URL instead of the signed object store URL where available.


IIUC @ywk253100, you are advocating that for some object stores, we would want to use the signed URL that is returned and not the download request when it is not needed?

If that is the case I agree, I wonder how we will determine that though?

shawn-hurley · 2024-04-18T19:04:04Z

design/velero-client-download-apiserver_design.md

+
+The Velero CLI will still default to `downloadURL` from DownloadRequest but can fallback or forced to `veleroServerURL` which is served via the Velero server.
+
+If velero CLI attempted request from veleroServerURL at the specified FQDN and failed, and velero CLI command was executed inside velero server pod (`alias velero='kubectl exec -it deployment/velero -- velero'`), velero CLI will attempt to use the service name (cluster.local address) to connect to the download server.


I am a little confused about when this is used. Is this just for documentation for a user to be able to access things if they don't have a velero CLI installed?

shawn-hurley · 2024-04-18T19:07:16Z

design/velero-client-download-apiserver_design.md

+}
+```
+### HTTPS Server Certificates
+Velero server will generate a self-signed certificate for HTTPS server endpoint. The certificate will be stored in a secret in the same namespace as the Velero server. The secret will be named `velero-download-server-certs`.


I would also explore when an ingress/router can use a CA cert and doesn't want to re-encrypt to talk to the service.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

kaovilai · 2024-04-24T01:15:18Z

Will be researching more info around cacerts for this design.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

ywk253100 · 2024-04-26T02:22:40Z

@kaovilai Do you think we should put the download server in a separate pod or in the same pod with Velero server? Could you also clarify this in the design?

kaovilai · 2024-04-26T02:28:01Z

I think a separate pod would be overkill in terms of resource requirements. Most of the time there's no download request to serve.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

github-actions bot added the Area/Design Design Documents label Jan 23, 2024

qiuming-best requested a review from ywk253100 January 29, 2024 07:09

kaovilai commented Jan 30, 2024

View reviewed changes

design/velero-client-download-apiserver_design.md Outdated Show resolved Hide resolved

github-actions bot added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jan 31, 2024

kaovilai mentioned this pull request Jan 31, 2024

Download server for Velero client #6167

Open

draghuram mentioned this pull request Mar 1, 2024

Add design for repository maintenance job #7375

Merged

3 tasks

kaovilai force-pushed the design-download-endpoint-for-client branch 4 times, most recently from acefb20 to ba6ff6f Compare March 28, 2024 21:34

kaovilai commented Mar 29, 2024

View reviewed changes

design/velero-client-download-apiserver_design.md Show resolved Hide resolved

ywk253100 reviewed Apr 1, 2024

View reviewed changes

design/velero-client-download-apiserver_design.md Outdated Show resolved Hide resolved

kaovilai force-pushed the design-download-endpoint-for-client branch 2 times, most recently from 732e235 to d9afec6 Compare April 1, 2024 18:45

kaovilai force-pushed the design-download-endpoint-for-client branch 3 times, most recently from 069f0c1 to c86e442 Compare April 8, 2024 20:22

kaovilai force-pushed the design-download-endpoint-for-client branch 9 times, most recently from fb4350f to 477db8f Compare April 17, 2024 03:19

kaovilai force-pushed the design-download-endpoint-for-client branch from 477db8f to 792a67f Compare April 17, 2024 03:21

kaovilai marked this pull request as ready for review April 17, 2024 03:24

github-actions bot assigned kaovilai Apr 17, 2024

github-actions bot requested review from reasonerjt and sseago April 17, 2024 03:25

ywk253100 reviewed Apr 18, 2024

View reviewed changes

shawn-hurley reviewed Apr 18, 2024

View reviewed changes

Remove Service ExternalName, add Ingress references

1a39871

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

kaovilai marked this pull request as draft April 24, 2024 01:15

Highlight direct download way is not going away

4fa20ef

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

Clarified ingress requirements

e2403f6

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

kaovilai force-pushed the design-download-endpoint-for-client branch from b0be065 to e2403f6 Compare May 17, 2024 20:47

updated abstract

119ac46

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design: Velero client download APIServer #7344

Design: Velero client download APIServer #7344

kaovilai commented Jan 23, 2024 •

edited

Loading

codecov bot commented Jan 23, 2024 •

edited

Loading

kaovilai commented Jan 31, 2024

ywk253100 Apr 17, 2024

shawn-hurley Apr 18, 2024

ywk253100 Apr 19, 2024

kaovilai Apr 23, 2024

kaovilai Apr 23, 2024

ywk253100 Apr 26, 2024

ywk253100 Apr 17, 2024

kaovilai Apr 18, 2024

shawn-hurley Apr 18, 2024

kaovilai Apr 18, 2024

kaovilai Apr 22, 2024

ywk253100 Apr 18, 2024

kaovilai Apr 26, 2024

ywk253100 Apr 26, 2024

kaovilai Apr 26, 2024

ywk253100 Apr 26, 2024

shawn-hurley Apr 26, 2024

ywk253100 Apr 27, 2024

ywk253100 Apr 18, 2024

shawn-hurley Apr 18, 2024

ywk253100 Apr 18, 2024

kaovilai Apr 26, 2024

shawn-hurley left a comment

shawn-hurley Apr 18, 2024

shawn-hurley Apr 18, 2024

shawn-hurley Apr 18, 2024

kaovilai commented Apr 24, 2024

ywk253100 commented Apr 26, 2024

kaovilai commented Apr 26, 2024


		This approach may also [work on KinD clusters](https://kind.sigs.k8s.io/docs/user/loadbalancer/)

		The current DownloadRequest CR status `veleroServerURL` field will be populated to use the Download Server URL instead of the signed object store URL where available.


		The Velero CLI will still default to `downloadURL` from DownloadRequest but can fallback or forced to `veleroServerURL` which is served via the Velero server.

		If velero CLI attempted request from veleroServerURL at the specified FQDN and failed, and velero CLI command was executed inside velero server pod (`alias velero='kubectl exec -it deployment/velero -- velero'`), velero CLI will attempt to use the service name (cluster.local address) to connect to the download server.


		The Velero CLI may use the VeleroServerURL to download data from the Velero server when DownloadURL is not populated, unreachable, or explicitly requested via `--veleroServerUrl`.

		When using VeleroServerURL, Velero CLI will send current kubeconfig to the download server via HTTP Authorization Header for velero server to authenticate and authorize the request.


		[![Velero Download Server](VeleroDownloadServer.jpg)](VeleroDownloadServer.jpg)

		## Detailed Design

Design: Velero client download APIServer #7344

Are you sure you want to change the base?

Design: Velero client download APIServer #7344

Conversation

kaovilai commented Jan 23, 2024 • edited Loading

Please add a summary of your change

Does your change fix a particular issue?

Please indicate you've done the following:

codecov bot commented Jan 23, 2024 • edited Loading

Codecov Report

kaovilai commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shawn-hurley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaovilai commented Apr 24, 2024

ywk253100 commented Apr 26, 2024

kaovilai commented Apr 26, 2024

kaovilai commented Jan 23, 2024 •

edited

Loading

codecov bot commented Jan 23, 2024 •

edited

Loading