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

fix(operator): Configure Loki to use virtual-host-style URLs for S3 AWS endpoints #12469

Merged
merged 16 commits into from
Apr 10, 2024

Conversation

btaani
Copy link
Contributor

@btaani btaani commented Apr 4, 2024

What this PR does / why we need it:
Change Loki's configuration to use virtual-host-style instead of path-style URLs when the object storage endpoint is AWS S3.

Which issue(s) this PR fixes:
Fixes LOG-5051

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 4, 2024
@btaani btaani changed the title Configure Loki to use virtual-host-style URLs for S3 AWS endpoints fix(operator): Configure Loki to use virtual-host-style URLs for S3 AWS endpoints Apr 4, 2024
Copy link
Collaborator

@xperimental xperimental left a comment

Choose a reason for hiding this comment

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

Did not test it yet with any cluster, but setting the s3forcepathstyle attribute based on whether we're actually using AWS is a nice workaround for not being able to break compatibility with other providers.

One general remark would be to put less of the logic into the go-template: instead of having a function and a conditional running in the template, move the force path style into an attribute of the context provided to the template and set the value from outside the template.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 9, 2024
operator/internal/manifests/config.go Outdated Show resolved Hide resolved
@periklis periklis enabled auto-merge (squash) April 10, 2024 08:04
@periklis periklis merged commit 0084262 into grafana:main Apr 10, 2024
17 of 18 checks passed
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants