Skip to content
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.

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

add capemconfig #828

Closed
wants to merge 4 commits into from
Closed

Conversation

data-dude
Copy link

/kind bug
/kind 4.19.0
/area falco-chart
What this PR does / why we need it:
If you don't set the caPEMBundle to spaces it causes tls issues

@poiana poiana added kind/bug Something isn't working dco-signoff: no labels Jan 31, 2025
@poiana
Copy link
Contributor

poiana commented Jan 31, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: data-dude
Once this PR has been reviewed and has the lgtm label, please assign issif for approval. For more information see the Kubernetes Code Review Process.

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

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

@poiana
Copy link
Contributor

poiana commented Jan 31, 2025

@data-dude: The label(s) kind/4.19.0 cannot be applied, because the repository doesn't have them.

In response to this:

/kind bug
/kind 4.19.0
/area falco-chart
What this PR does / why we need it:
If you don't set the caPEMBundle to spaces it causes tls issues

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.

@poiana poiana requested review from bencer and leogr January 31, 2025 16:58
@poiana poiana added the size/XS label Jan 31, 2025
@leogr
Copy link
Member

leogr commented Feb 5, 2025

Hey @data-dude

Can you sign off your commit, please?
Also, can you elaborate more on the issue this PR aims to address?

@poiana
Copy link
Contributor

poiana commented Feb 5, 2025

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

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.

@data-dude
Copy link
Author

data-dude commented Feb 5, 2025

If I didn't set caPEMBundle to spaces I ran into ssl errors with k8s-metacollector. And there's no way to set the caPEMBundle. This change fixed it for me. By default k8s-metacollector doesn't use ssl but it seems the plugin assumes you are using ssl if caPEMBundle isn't set to spaces.

I just now signed the branch change.
Change in values.yaml and _helpers.tpl
And regardless there's no way to set the caPEMBundle without this change, I believe.

@poiana
Copy link
Contributor

poiana commented Feb 5, 2025

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

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.

@data-dude
Copy link
Author

close this and create a new one

@data-dude data-dude closed this Feb 5, 2025
@data-dude data-dude deleted the addcapemconfig branch February 5, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants