dskit icon indicating copy to clipboard operation
dskit copied to clipboard

Published 20 hours ago • verified publisher icon grafana

flagext: Add new type `URLEscaped`

Open kavirajk opened this issue 2 years ago • 2 comments

What this PR does: Sometimes we need to store just escaped URL in the config like S3 URL that contains credentials which can include characters like : and /. Currently we cannot use normal flagext.URLValue as it just uses unescaped URL.

Please check issue https://github.com/grafana/loki/issues/1607

Which issue(s) this PR fixes:

Fixes https://github.com/grafana/loki/issues/1607

Checklist

  • [x] Tests updated
  • [ ] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

kavirajk avatar Feb 07 '23 21:02 kavirajk

Is this work around for properly formatting strings in YAML file?

pstibrany avatar Feb 08 '23 07:02 pstibrany

Is this work around for properly formatting strings in YAML file?

Kinda. So basically user should be able to give unescaped S3 bucket URL in both yaml config and as a CLI flag.

Our internal dependency aws.ConfigFromURL expects escaped URL always (from weaveworks/common).

Trying to transparenly handle this case. But realized it's hard to do it in backward compatible way. e.g: If people already working with escaped URL. Tried few things but looks bit tricky. Converting to draft PR till I find right way to do it.

kavirajk avatar Feb 08 '23 08:02 kavirajk