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]

Reply via email to