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

rm chart value 'allowNamespaceDiscovery' fixing issue #5422 #5574

Merged
merged 4 commits into from
Oct 29, 2022
Merged

rm chart value 'allowNamespaceDiscovery' fixing issue #5422 #5574

merged 4 commits into from
Oct 29, 2022

Conversation

giulianopz
Copy link
Contributor

Fix issue #5422.

@netlify
Copy link

netlify bot commented Oct 26, 2022

Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 30eef51
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/635d23de201d190008c4a569

@vmwclabot
Copy link

@giulianopz, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@vmwclabot
Copy link

@giulianopz, we have received your signed contributor license agreement. It will be reviewed by VMware shortly. Another comment will be added to the pull request to notify you when the merge can proceed.

@antgamdia
Copy link
Contributor

Thanks for your contribution, @giulianopz . Please, have a look at this project we are using to auto-generate the readme: https://github.com/bitnami-labs/readme-generator-for-helm. Then, run it and update the README.md with the change you performed in the values.yaml file.
Also, please bump up the chart version to sth like, x.x.x-devN+1, for instance, 11.0.1-dev2

Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Thanks for using the tool I suggested to autogenerate the readme file. Find below just a couple of minor changes before merging the PR (mainly removing some unwanted changes). Apart from that, +1.

Also, a github action check is currently failing; we are working to fix it, it's not your PR's fault :P

Comment on lines 698 to 728
- [Kubeapps packaged by Bitnami](#kubeapps-packaged-by-bitnami)
- [TL;DR](#tldr)
- [Introduction](#introduction)
- [Prerequisites](#prerequisites)
- [Installing the Chart](#installing-the-chart)
- [Parameters](#parameters)
- [Global parameters](#global-parameters)
- [Common parameters](#common-parameters)
- [Traffic Exposure Parameters](#traffic-exposure-parameters)
- [Kubeapps packaging options](#kubeapps-packaging-options)
- [Frontend parameters](#frontend-parameters)
- [Dashboard parameters](#dashboard-parameters)
- [AppRepository Controller parameters](#apprepository-controller-parameters)
- [Auth Proxy parameters](#auth-proxy-parameters)
- [Pinniped Proxy parameters](#pinniped-proxy-parameters)
- [Other Parameters](#other-parameters)
- [Feature flags](#feature-flags)
- [Database Parameters](#database-parameters)
- [kubeappsapis parameters](#kubeappsapis-parameters)
- [Redis® chart configuration](#redis-chart-configuration)
- [Configuration and installation details](#configuration-and-installation-details)
- [Configuring Initial Repositories](#configuring-initial-repositories)
- [Enabling Operators](#enabling-operators)
- [Exposing Externally](#exposing-externally)
- [LoadBalancer Service](#loadbalancer-service)
- [Ingress](#ingress)
- [Hosts](#hosts)
- [Annotations](#annotations)
- [TLS](#tls)
- [Upgrading Kubeapps](#upgrading-kubeapps)
- [Uninstalling the Chart](#uninstalling-the-chart)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to add these changes, we only want to keep the FAQ section, not the whole table of contents of the readme at this point. Please, revert these changes

@@ -1,125 +1,2886 @@
{
"$schema": "http://json-schema.org/schema#",
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for using the tool to also generate the underlying JSON schema. That said, currently we don't want to use this schema. Just for you to know, in the latest v2.6.0 release, we migrated to full json schema based form. Along with this chance, the Bitnami Application Catalog is gonna autogenerate the schemas using this tool soon. Hence, we want to switch the schema once they also perform the change. Hope it makes sense for you.

In short, please revert the changes in this file.

@giulianopz
Copy link
Contributor Author

Thanks for using the tool I suggested to autogenerate the readme file. Find below just a couple of minor changes before merging the PR (mainly removing some unwanted changes). Apart from that, +1.

Also, a github action check is currently failing; we are working to fix it, it's not your PR's fault :P

Unwanted changes reverted as requested.
Actually, thanks for suggesting that tool. I didn't know it: very useful.

Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Great, thanks for your contribution :)

@antgamdia
Copy link
Contributor

@giulianopz , there's an additional pending thing. According to our contributing guidelines, all the commits must be signed.
That this is twofold: 1) sign them with GPG [1]; 2) add the DCO in each commit's msg [2]. You might need to force push the changes to your branch in order to replace the non-signed commits.

If you face any problems, happy to assist via slack. Let's try to get it to merge before the end of October, so that it can count for the hacktoberfest!

[1] https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits
[2] https://wiki.linuxfoundation.org/dco

Signed-off-by: giuliano <panzironi.giuliano@gmail.com>
Signed-off-by: giuliano <panzironi.giuliano@gmail.com>
Signed-off-by: giuliano <panzironi.giuliano@gmail.com>
Signed-off-by: giuliano <panzironi.giuliano@gmail.com>
@giulianopz
Copy link
Contributor Author

@giulianopz , there's an additional pending thing. According to our contributing guidelines, all the commits must be signed. That this is twofold: 1) sign them with GPG [1]; 2) add the DCO in each commit's msg [2]. You might need to force push the changes to your branch in order to replace the non-signed commits.

If you face any problems, happy to assist via slack. Let's try to get it to merge before the end of October, so that it can count for the hacktoberfest!

[1] https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits [2] https://wiki.linuxfoundation.org/dco

Never done before. Happy that contributing to a simple issue taught me a couple of things. Thanks!

@antgamdia
Copy link
Contributor

Thank you! LGTM, merging straight away :)

@antgamdia antgamdia merged commit d20d2b2 into vmware-tanzu:main Oct 29, 2022
@vmwclabot
Copy link

@giulianopz, VMware has rejected your signed contributor license agreement. The change has already been merged, but it will be backed out by the project maintainers if the agreement is not resigned in a timely manner. Click here to resign the agreement.

@vmwclabot
Copy link

@giulianopz, we have received your signed contributor license agreement. It will be reviewed by VMware shortly. Another comment will be added to the pull request to notify you when the merge can proceed.

@vmwclabot
Copy link

@giulianopz, VMware has approved your signed contributor license agreement.

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

Successfully merging this pull request may close these issues.

3 participants