Skip to content

OCPBUGS-29804: Fix crash if helm chart metadata is nil#13963

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
christoph-jerolimov:fix-helm-crash
Jun 13, 2024
Merged

OCPBUGS-29804: Fix crash if helm chart metadata is nil#13963
openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
christoph-jerolimov:fix-helm-crash

Conversation

@christoph-jerolimov
Copy link
Member

@christoph-jerolimov christoph-jerolimov commented Jun 12, 2024

Fixes:
Fixes an internal crash related to https://issues.redhat.com/browse/OCPBUGS-29804 without updating the helm library.

Related to #13816

I added just one new nil check:

if entries[i] == nil || entries[i].Metadata == nil {
  klog.Warningf("Helm chart %v from repository %v has an invalid entry at index %v", key, helmRepo.Name, i)
  entries = append(entries[:i], entries[i+1:]...)
  continue
}

The rest of the change is only because I renamed the array entry to entries.

Test setup:

  1. Open the console and switch to the developer perspective
  2. Navigate to Helm
  3. Add this helm repository: https://jerolimov.github.io/helm-cve-2024-26147-test/
  4. Click Create > Helm Release to search for available helm charts.

Previously the page showed an error.

With this change the page works fine also with this broken helm repository.

@openshift-ci-robot
Copy link
Contributor

@jerolimov: An error was encountered searching for bug OCPBUGS-29804 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message. You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 401:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

Details

In response to this:

Fixes an internal crash related to https://issues.redhat.com/browse/OCPBUGS-29804 without updating the helm library.

Related to #13816

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@christoph-jerolimov
Copy link
Member Author

/jira refresh
/cc @Kartikey-star @jhadvig
/cherry-pick release-4.16 release-4.15 release-4.14

@openshift-cherrypick-robot

@jerolimov: once the present PR merges, I will cherry-pick it on top of release-4.16 in a new PR and assign it to you.

Details

In response to this:

/jira refresh
/cc @Kartikey-star @jhadvig
/cherry-pick release-4.16 release-4.15 release-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot requested review from Kartikey-star and jhadvig June 12, 2024 11:21
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Jun 12, 2024
@openshift-ci-robot
Copy link
Contributor

@jerolimov: This pull request references Jira Issue OCPBUGS-29804, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.17.0) matches configured target version for branch (4.17.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @yapei

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

/jira refresh
/cc @Kartikey-star @jhadvig
/cherry-pick release-4.16 release-4.15 release-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from dperaza4dustbit and yapei June 12, 2024 11:21
@openshift-ci openshift-ci bot added component/backend Related to backend approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 12, 2024
Comment on lines +98 to 102
if entries[i] == nil || entries[i].Metadata == nil {
klog.Warningf("Helm chart %v from repository %v has an invalid entry at index %v", key, helmRepo.Name, i)
entries = append(entries[:i], entries[i+1:]...)
continue
}
Copy link
Member Author

@christoph-jerolimov christoph-jerolimov Jun 12, 2024

Choose a reason for hiding this comment

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

This is actually the 5 new lines.

The rest is only because I renamed the array entry to entries.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@Kartikey-star Kartikey-star left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 12, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerolimov, Kartikey-star

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD f9d3cf5 and 2 for PR HEAD 28c3d57 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 13, 2024

@jerolimov: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 15985cf into openshift:master Jun 13, 2024
@openshift-ci-robot
Copy link
Contributor

@jerolimov: Jira Issue OCPBUGS-29804: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-29804 has been moved to the MODIFIED state.

Details

In response to this:

Fixes:
Fixes an internal crash related to https://issues.redhat.com/browse/OCPBUGS-29804 without updating the helm library.

Related to #13816

I added just one new nil check:

if entries[i] == nil || entries[i].Metadata == nil {
 klog.Warningf("Helm chart %v from repository %v has an invalid entry at index %v", key, helmRepo.Name, i)
 entries = append(entries[:i], entries[i+1:]...)
 continue
}

The rest of the change is only because I renamed the array entry to entries.

Test setup:

  1. Open the console and switch to the developer perspective
  2. Navigate to Helm
  3. Add this helm repository: https://jerolimov.github.io/helm-cve-2024-26147-test/
  1. Click Create > Helm Release to search for available helm charts.

Previously the page showed an error.

With this change the page works fine also with this broken helm repository.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-cherrypick-robot

@jerolimov: #13963 failed to apply on top of branch "release-4.16":

Applying: Fix crash if helm chart metadata is nil
Using index info to reconstruct a base tree...
M	pkg/helm/chartproxy/proxy.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/helm/chartproxy/proxy.go
CONFLICT (content): Merge conflict in pkg/helm/chartproxy/proxy.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix crash if helm chart metadata is nil
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/jira refresh
/cc @Kartikey-star @jhadvig
/cherry-pick release-4.16 release-4.15 release-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@christoph-jerolimov christoph-jerolimov deleted the fix-helm-crash branch June 14, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/backend Related to backend jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants