lhotari commented on PR #448: URL: https://github.com/apache/pulsar-helm-chart/pull/448#issuecomment-1919052024
> I updated with your feedback and decided to also reduce the complexity of the secret by only use a single username/password for the UI / DB. I don't see much advantage of having separate ones there. I think having a single username/password for both UI & DB is simply a very bad practice. It's better to have separate passwords. It's not that much additional overhead to add it now and follow good practices. > I allowed for manual overwrite of secret values, similar to kube-prometheus-stack, but the default (empty password) is a randomly generated password. That part looks really good! > Maybe you have an idea how can test if the login/checking of the environment of the pulsar manager works in the CI pipeline? Perhaps a simple HTTP request with curl & grep would do the job? That could be added in a similar way as the Pulsar Functions tests: https://github.com/apache/pulsar-helm-chart/blob/24b80c19865e2d9e0285c54dca05835bb783c8ee/.ci/chart_test.sh#L77-L80 You would add this to `.ci/chart_test.sh` and `.ci/helm.sh` files. In addition there would need to be a test scenario in the GitHub Actions workflow matrix configured in https://github.com/apache/pulsar-helm-chart/blob/master/.github/workflows/pulsar-helm-chart-ci.yaml#L170 with respective values file in .ci/clusters directory. When you run the CI jobs in your own fork (by enabling GitHub Actions) and opening a PR to your own fork, you can debug GitHub Actions by logging in by ssh. ssh access is only for your ssh public keys registered in GH, it's https://github.com/Mortom123.keys for you. There's a step in the build that will print the command for connecting with ssh. [example from a build in my fork](https://github.com/lhotari/pulsar-helm-chart/actions/runs/7670897358/job/20908119338#step:5:156): <img width="975" alt="image" src="https://github.com/apache/pulsar-helm-chart/assets/66864/a3b84c2c-a337-464e-b2eb-a2ad31c5fcf9"> Once you have ssh access, you can debug things to see what problems there are in the CI environment. > I removed existingSecretName, because when injecting Secrets through secondary applications, I would argue that you have to provide a name for it either way. so might as well set it to the required name. +1, pulsar-manager integration has been poor so I'm not concerned about the possible breaking change. > During this I also wondered why we have such flexibility for the names for of the components. Most of the things are setup using pulsar.fullname-component.name. Why don't we fix component.name and only allow for a changing fullname through e.g. other release names? I guess this is some of the legacy baggage of the chart. As long as there's a backwards compatible way to improve things gradually, we can do that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
