[prometheus-pushgateway] Allow existing secret for web config#6624
Conversation
Signed-off-by: Firas Mosbehi <firas.mosbehi@insat.ucar.tn>
Signed-off-by: Firas Mosbehi <firas.mosbehi@insat.ucar.tn>
Signed-off-by: Firas Mosbehi <firas.mosbehi@insat.ucar.tn>
zeritti
left a comment
There was a problem hiding this comment.
Thak you, @firasmosbehi, for your PR. Please, see my comments below.
| webConfiguration: {} | ||
| # basicAuthUsers: | ||
| # username: password | ||
| # existingSecret: |
There was a problem hiding this comment.
The user needs to know that the secret's key must be web-config.yaml as this name is expected in the corresponding Pushgateway's flag.
The feature itself is useful. However, setting existingSecret will lead to Pushgateway never becoming ready being repeatedly restarted. This is because probes have no knowledge of the basic authentication configuration as long as it is present in the web-config file (would receive 401). The user would have to know the configuration and enter it in the probes' headers handling the values properly as secrets. I think we should not require the user to do that as this would go against the advantage of using an existing secret.
There was a problem hiding this comment.
@firasmosbehi Hello! Are you planning to make the changes mentioned?
There was a problem hiding this comment.
To prevent an exposure of the credentials in probes' configuration, we could switch from the httpGet probe to the tcpSocket probe if an existing secret is set. I am not aware of any negative impact of such a change in this case. The only thing that might not be "nice" would probably be logging TLS handshake errors at Pushgateway with TLS set up.
What this PR does
webConfiguration.existingSecret.namesupport so users can provide an existing web config secret.webConfiguration.basicAuthUsers.Fixes #5979.
Testing