feat(backend) Enable auth between pesistence agent and pipelineAPI (ReportServer) #9699

difince · 2023-07-03T12:37:25Z

Add authentication and authorization logic to PipelineAPI ReportServer's methods - reportworkflow & reportSheduledWorkflow and
Force Persistence Agent to authenticate itself through Service Account Token Volume Projection when calling the above methods.

Issue: #8074

Description of your changes:

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

difince · 2023-07-04T07:06:13Z

/retest

chensun · 2023-07-05T22:42:42Z

/retest

chensun · 2023-07-06T16:52:07Z

backend/src/agent/persistence/client/pipeline_client.go

+		} else if statusCode.Code() == codes.Unauthenticated && strings.Contains(err.Error(), "service account token has expired") {
+			//if unauthenticated because SA token is expired, re-read/refresh the token and try again
+			p.tokenRefresher.RefreshToken()
+			return util.NewCustomError(err, util.CUSTOM_CODE_TRANSIENT,


Although I see you're following an existing pattern, for my education how would user discover such error? Would this be surfaced back by apiserver?

Hi! Sorry for my late response.
The user could observe the status of the Run and inspect the logs of the persistent agent. The errors are not directly displayed on the UI.
If a "transient" error occurs the Persistent Agent will retry with an exponential delay

Here, If an error occurs on token refresh, I just log the error and let the worker thread retries. Any other suggestions on what to do if the refresh token fails?

backend/src/agent/persistence/client/token_refresher.go

manifests/kustomize/base/pipeline/ml-pipeline-persistenceagent-deployment.yaml

backend/src/apiserver/server/report_server.go

zijianjoy · 2023-07-13T22:38:06Z

/assign @chensun

1. Add authentication and authorization logic to PipelineAPI's PeportServer & 2. Make Persistence Agent authenticate itself through Service Account Token Volume Projection. Signed-off-by: diana <[email protected]>

Only Persistent agent can KFPipeline API "Report (Scheduled)Workflows" no matter the namespace workflows belongs to. Signed-off-by: diana <[email protected]>

juliusvonkohout · 2023-08-04T09:32:46Z

@difince is any help needed?

- unit tests added - do not stop the ticker on stopCh Signed-off-by: diana <[email protected]>

difince · 2023-08-16T14:11:38Z

backend/src/agent/persistence/client/token_refresher.go

+
+	ticker := time.NewTicker(*tr.seconds)
+	go func() {
+		for range ticker.C {


I was unsure about the use of the stopCh, so I removed it. WDYT? Please inspect the changes introduced by my third commit.

chensun

/lgtm
/approve

Thanks!

google-oss-prow · 2023-08-16T18:22:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~backend/OWNERS~~ [chensun]
~~manifests/kustomize/OWNERS~~ [chensun]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chensun · 2023-08-16T18:22:56Z

I'm investigating the test failure, which is likely unrelated to this change.
#9883

juliusvonkohout · 2023-08-17T10:55:48Z

/retest-required

juliusvonkohout · 2023-08-17T10:56:59Z

@difince maybe you have to rebase to #9885 if the test fails again

…eportServer) (#9699) * Enable auth between pesistence agent and pipelineAPI (ReportServer) 1. Add authentication and authorization logic to PipelineAPI's PeportServer & 2. Make Persistence Agent authenticate itself through Service Account Token Volume Projection. Signed-off-by: diana <[email protected]> * Do not use MULTIUSER on report weorkflows Only Persistent agent can KFPipeline API "Report (Scheduled)Workflows" no matter the namespace workflows belongs to. Signed-off-by: diana <[email protected]> * Add unit tests - unit tests added - do not stop the ticker on stopCh Signed-off-by: diana <[email protected]> --------- Signed-off-by: diana <[email protected]>

…eportServer) (kubeflow#9699) * Enable auth between pesistence agent and pipelineAPI (ReportServer) 1. Add authentication and authorization logic to PipelineAPI's PeportServer & 2. Make Persistence Agent authenticate itself through Service Account Token Volume Projection. Signed-off-by: diana <[email protected]> * Do not use MULTIUSER on report weorkflows Only Persistent agent can KFPipeline API "Report (Scheduled)Workflows" no matter the namespace workflows belongs to. Signed-off-by: diana <[email protected]> * Add unit tests - unit tests added - do not stop the ticker on stopCh Signed-off-by: diana <[email protected]> --------- Signed-off-by: diana <[email protected]>

google-oss-prow bot added the size/L label Jul 3, 2023

google-oss-prow bot requested review from gkcalat and zijianjoy July 3, 2023 12:38

difince force-pushed the auth_report_server branch from 49e7874 to e446cc9 Compare July 5, 2023 12:46

chensun reviewed Jul 6, 2023

View reviewed changes

difince commented Jul 7, 2023

View reviewed changes

manifests/kustomize/base/pipeline/ml-pipeline-persistenceagent-deployment.yaml Show resolved Hide resolved

backend/src/apiserver/server/report_server.go Outdated Show resolved Hide resolved

difince commented Jul 7, 2023

View reviewed changes

backend/src/apiserver/server/report_server.go Show resolved Hide resolved

google-oss-prow bot assigned chensun Jul 13, 2023

difince added 2 commits July 18, 2023 14:37

Enable auth between pesistence agent and pipelineAPI (ReportServer)

5c0df51

1. Add authentication and authorization logic to PipelineAPI's PeportServer & 2. Make Persistence Agent authenticate itself through Service Account Token Volume Projection. Signed-off-by: diana <[email protected]>

Do not use MULTIUSER on report weorkflows

1de9bd0

Only Persistent agent can KFPipeline API "Report (Scheduled)Workflows" no matter the namespace workflows belongs to. Signed-off-by: diana <[email protected]>

difince force-pushed the auth_report_server branch from 8635d29 to 1de9bd0 Compare July 18, 2023 11:38

difince force-pushed the auth_report_server branch 2 times, most recently from 4333fc2 to 19e6d60 Compare August 16, 2023 13:48

Add unit tests

bc9e1e0

- unit tests added - do not stop the ticker on stopCh Signed-off-by: diana <[email protected]>

difince force-pushed the auth_report_server branch from 19e6d60 to bc9e1e0 Compare August 16, 2023 14:02

difince commented Aug 16, 2023

View reviewed changes

chensun approved these changes Aug 16, 2023

View reviewed changes

google-oss-prow bot added the lgtm label Aug 16, 2023

google-oss-prow bot added the approved label Aug 16, 2023

google-oss-prow bot merged commit cb18d00 into kubeflow:master Aug 17, 2023
1 check passed

difince mentioned this pull request Aug 17, 2023

[backend] Persistent agent SA token Refresher does wrong time conversion #9891

Closed

difince mentioned this pull request Aug 17, 2023

[feature] Add authorization to all functions in ReportServer #8074

Closed

difince mentioned this pull request Aug 28, 2023

[feature] Make Persistence Agent use SA Token when calling KF APIServer endpoints #9937

Closed

difince mentioned this pull request Sep 4, 2023

feat(backend): enforce SA Token based auth b/w Persistence Agent and Pipeline API Server #9957

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(backend) Enable auth between pesistence agent and pipelineAPI (ReportServer) #9699

feat(backend) Enable auth between pesistence agent and pipelineAPI (ReportServer) #9699

difince commented Jul 3, 2023 •

edited

Loading

difince commented Jul 4, 2023

chensun commented Jul 5, 2023

chensun Jul 6, 2023

difince Jul 12, 2023

difince Aug 16, 2023

zijianjoy commented Jul 13, 2023

juliusvonkohout commented Aug 4, 2023

difince Aug 16, 2023 •

edited

Loading

chensun left a comment

google-oss-prow bot commented Aug 16, 2023

chensun commented Aug 16, 2023

juliusvonkohout commented Aug 17, 2023

juliusvonkohout commented Aug 17, 2023

feat(backend) Enable auth between pesistence agent and pipelineAPI (ReportServer) #9699

feat(backend) Enable auth between pesistence agent and pipelineAPI (ReportServer) #9699

Conversation

difince commented Jul 3, 2023 • edited Loading

difince commented Jul 4, 2023

chensun commented Jul 5, 2023

chensun Jul 6, 2023

Choose a reason for hiding this comment

difince Jul 12, 2023

Choose a reason for hiding this comment

difince Aug 16, 2023

Choose a reason for hiding this comment

zijianjoy commented Jul 13, 2023

juliusvonkohout commented Aug 4, 2023

difince Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

chensun left a comment

Choose a reason for hiding this comment

google-oss-prow bot commented Aug 16, 2023

chensun commented Aug 16, 2023

juliusvonkohout commented Aug 17, 2023

juliusvonkohout commented Aug 17, 2023

difince commented Jul 3, 2023 •

edited

Loading

difince Aug 16, 2023 •

edited

Loading