Skip to content

Close reponse right after succesful request #8894

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 13, 2023
Merged

Close reponse right after succesful request #8894

merged 1 commit into from
Nov 13, 2023

Conversation

AdamKorcz
Copy link
Contributor

Explanation

Moves the closing of the response up. Currently the response will not be closed if Kyverno fails this conditional:

kyverno/pkg/engine/apicall/apiCall.go

Lines 131 to 138 in 9c3be3f

if resp.StatusCode < 200 || resp.StatusCode >= 300 {
b, err := io.ReadAll(resp.Body)
if err == nil {
return nil, fmt.Errorf("HTTP %s: %s", resp.Status, string(b))
}
return nil, fmt.Errorf("HTTP %s", resp.Status)
}

This PR fixes that.

Related issue

NONE

What type of PR is this

/kind bug

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

@AdamKorcz
Close reponse right after succesful request
c7619fd 
Signed-off-by: AdamKorcz <adam@adalogics.com>
@codecov Codecov
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #8894 (c7619fd) into main (9c3be3f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #8894   +/-   ##
=======================================
  Coverage   33.74%   33.74%           
=======================================
  Files         314      314           
  Lines       24811    24812    +1     
=======================================
+ Hits         8373     8374    +1     
  Misses      15643    15643           
  Partials      795      795
Files Coverage Δ
pkg/engine/apicall/apiCall.go 47.36% <100.00%> (+0.30%) ⬆️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

realshuting
realshuting approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

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

/lgtm

@realshuting realshuting enabled auto-merge (squash) November 13, 2023 13:30
@realshuting realshuting merged commit 4da9633 into kyverno:main Nov 13, 2023
@realshuting
Copy link
Member

/cherry-pick release-1.11

@gcp-cherry-pick-bot gcp-cherry-pick-bot bot mentioned this pull request Nov 13, 2023
Merged
gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Nov 13, 2023
@AdamKorcz
Close reponse right after succesful request (#8894)
a05f4ce 
Signed-off-by: AdamKorcz <adam@adalogics.com>
@realshuting realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label Nov 13, 2023
@realshuting
Copy link
Member

Nice catch @AdamKorcz !

realshuting pushed a commit that referenced this pull request Nov 13, 2023
@gcp-cherry-pick-bot @AdamKorcz
Close reponse right after succesful request (#8894) (#8896)
1fc15cc 
Signed-off-by: AdamKorcz <adam@adalogics.com>
Co-authored-by: AdamKorcz <44787359+AdamKorcz@users.noreply.github.com>
rds13 pushed a commit to Deveryware/kyverno that referenced this pull request Nov 13, 2023
@AdamKorcz @rds13
Close reponse right after succesful request (kyverno#8894)
2cabc31 
Signed-off-by: AdamKorcz <adam@adalogics.com>
vishal-chdhry pushed a commit to vishal-chdhry/kyverno that referenced this pull request Jan 5, 2024
@AdamKorcz @vishal-chdhry
Close reponse right after succesful request (kyverno#8894)
dd5749c 
Signed-off-by: AdamKorcz <adam@adalogics.com>
vishal-chdhry pushed a commit to vishal-chdhry/kyverno that referenced this pull request Jan 5, 2024
@AdamKorcz @vishal-chdhry
Close reponse right after succesful request (kyverno#8894)
de2ccd4 
Signed-off-by: AdamKorcz <adam@adalogics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@realshuting realshuting realshuting approved these changes

@MarcelMue MarcelMue Awaiting requested review from MarcelMue

@MariamFahmy98 MariamFahmy98 Awaiting requested review from MariamFahmy98 MariamFahmy98 is a code owner

Assignees
No one assigned
Labels
cherry-pick-completed The PR was cherry-picked (or merged) to required release branches cherry-pick-required milestone 1.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AdamKorcz @realshuting