Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

fix: Use resourceHandler from apiv2 package instead of newly creating an internal-only, unauthenticated client #535

Merged
merged 7 commits into from
Aug 31, 2022

Conversation

bacherfl
Copy link
Member

@bacherfl bacherfl commented Aug 24, 2022

Closes keptn/keptn/#8722

This PR Introduces a new APIV2() method to the Keptn SDK's Keptn Handler, which retrieves the recently introduced v2 API client. The returned API handler will either use the in-cluster mode (directly calling the internal service URLs of the control-plane's API), or send its requests with an x-token via the Keptn Gateway's API endpoint. As previously, this is determined by setting the environment variables KEPTN_API_ENDPOINT and KEPTN_API_TOKEN.
For backwards compatibility, the APIV1() method is still there, but has been deprecated.

This PR also fixes the GetResourceHandler() method of the SDKs Keptn Handler by returning a wrapper to the previously initialized v2 Resource API client, as opposed to a resource handler that is only usable in in-cluster mode. The signature of the resource handler's GetResource() method has not been changed in this PR, so no adaptation should be required for services that were already using this.

…h proper api authentication

Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@5ff2d40). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #535   +/-   ##
=========================================
  Coverage          ?   50.44%           
=========================================
  Files             ?      105           
  Lines             ?     5792           
  Branches          ?        0           
=========================================
  Hits              ?     2922           
  Misses            ?     2737           
  Partials          ?      133           

Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
@bacherfl bacherfl marked this pull request as ready for review August 25, 2022 06:44
@christian-kreuzberger-dtx
Copy link
Member

FYI @TannerGabriel please give it a try, and let us know if this works

Signed-off-by: Florian Bacher <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Aug 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@TannerGabriel TannerGabriel left a comment

Choose a reason for hiding this comment

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

I tried it out in the job-executor-service. The remote execution plane file fetching seems to work fine now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants