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.

Sign up for GitHub

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

Jump to bottom

[bitnami/thanos] Add support for HTTPS and basic auth experimental settings #12404

Merged
merged 8 commits into from
Sep 14, 2022
Merged

[bitnami/thanos] Add support for HTTPS and basic auth experimental settings #12404

merged 8 commits into from
Sep 14, 2022

Conversation

migruiz4
Copy link
Member

Description of the change

Adds support for Thanos HTTPS and basic authentication.

Ref: https://thanos.io/tip/operating/https.md/#running-thanos-with-https-and-basic-authentication

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@migruiz4 migruiz4 added the verify Execute verification workflow for these changes label Sep 13, 2022
@migruiz4
Copy link
Member Author

I have added a new section in the README.md describing how to use this new feature.

@migruiz4
Copy link
Member Author

migruiz4 commented Sep 14, 2022 •
edited
Loading

In case it helps, this PR has been tested using the following install commands:

helm install test-all bitnami/thanos --set https.enabled=true --set https.autoGenerated=true --set auth.basicAuthUsers.admin=myadminpass
helm install test-auth bitnami/thanos --set auth.basicAuthUsers.admin=myadminpass
helm install test-https bitnami/thanos --set https.enabled=true --set https.autoGenerated=true

Output:

root@test:/# curl -k https://test-all-thanos-query:9090/-/healthy
Unauthorized
root@test:/# curl http://test-auth-thanos-query:9090/-/healthy
Unauthorized
root@test:/# curl -k https://test-https-thanos-query:9090/-/healthy
OK
root@test:/#

@migruiz4
Fix minor issue
22a50c7 
Signed-off-by: Miguel Ruiz <[email protected]>
@migruiz4
Fix typo
cdb9bd0 
Signed-off-by: Miguel Ruiz <[email protected]>
rafariossaa
rafariossaa suggested changes Sep 14, 2022
View reviewed changes
bitnami/thanos/README.md
| `https.cert` | TLS Certificate for Thanos HTTPS - ignored if existingSecret is provided | `""` |
| `https.ca` | (Optional, used for client) CA Certificate for Thanos HTTPS - ignored if existingSecret is provided | `""` |
| `https.clientAuthType` | Server policy for client authentication using certificates. Maps to ClientAuth Policies. | `""` |
| `auth.basicAuthUsers` | Object containing <user>:<passwords> key-value pairs for each user that will have access via basic authentication | `{}` |
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is containing passwords, I think we should use a secret, or at least, provide the option to use a secret instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

The configuration file is stored in a Secret instead of a configmap because of that reason.

User can provide its configuration file using existingHttpConfigSecret.

The main issue is that, if the basic authentication is enabled, the password is also needed for the Probes to succeed, and Thanos uses a scratch container, so no logic can be added to handle the password using env variables.

Copy link
Member Author

@migruiz4 migruiz4 Sep 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Please refer to the README.md section below, where I tried to depict this same situation and the alternatives to that.

javsalgar
javsalgar requested changes Sep 14, 2022
View reviewed changes
bitnami/thanos/templates/_helpers.tpl Outdated
{{/*
Returns Thanos basic auth user and password for the HTTP request.
*/}}
{{- define "thanos.basicAuth" -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this part as it is no longer necessary

@migruiz4
Fix typo
7a0d83a 
Signed-off-by: Miguel Ruiz <[email protected]>
@migruiz4 migruiz4 merged commit 422981c into bitnami:master Sep 14, 2022
isantospardo added a commit to adfinis/helm-charts that referenced this pull request Sep 30, 2022
@isantospardo
feat(thanos): Update thanos to 11.5.*
e8183ba 
- [Creates separate service for grpc port of query module](bitnami/charts#11051)
- [Update default prometheusrule value](bitnami/charts#10979)
- [Fix Deprecation Warning of thanos](bitnami/charts#11178)
- [Updating components versions](bitnami/charts@a49e568)
- [Update URLs to point to the new bitnami/containers monorepo](bitnami/charts#11352)
- [Conditionally Set objstore arg and OBJSTORE_CONFIG for Thanos receive](bitnami/charts#11274)
- [Add support for image digest apart from tag](bitnami/charts#11955)
- [Create sharded hpa and pdb for storegateway](bitnami/charts#11426)
- [Allowed to add labels to query-frontend service and storegateway PVC](bitnami/charts#11549)
- [Add support for HTTPS and basic auth experimental settings](bitnami/charts#12404)
@isantospardo isantospardo mentioned this pull request Sep 30, 2022
Merged
6 tasks
isantospardo added a commit to adfinis/helm-charts that referenced this pull request Sep 30, 2022
@isantospardo
feat(thanos): Update thanos to 11.5.* (#821)
03d9794 
- [Creates separate service for grpc port of query module](bitnami/charts#11051)
- [Update default prometheusrule value](bitnami/charts#10979)
- [Fix Deprecation Warning of thanos](bitnami/charts#11178)
- [Updating components versions](bitnami/charts@a49e568)
- [Update URLs to point to the new bitnami/containers monorepo](bitnami/charts#11352)
- [Conditionally Set objstore arg and OBJSTORE_CONFIG for Thanos receive](bitnami/charts#11274)
- [Add support for image digest apart from tag](bitnami/charts#11955)
- [Create sharded hpa and pdb for storegateway](bitnami/charts#11426)
- [Allowed to add labels to query-frontend service and storegateway PVC](bitnami/charts#11549)
- [Add support for HTTPS and basic auth experimental settings](bitnami/charts#12404)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javsalgar javsalgar javsalgar approved these changes

@rafariossaa rafariossaa rafariossaa approved these changes

Assignees

@rafariossaa rafariossaa

Labels
bitnami solved thanos verify Execute verification workflow for these changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@migruiz4 @javsalgar @rafariossaa @carrodher @bitnami-bot