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

Add prometheus namespace validation #519

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Add prometheus namespace validation #519

merged 1 commit into from
Nov 14, 2023

Conversation

Choraden
Copy link
Contributor

go run ./cmd/forwarder run --prom-namespace upstream-proxy
Error: invalid argument "upstream-proxy" for "--prom-namespace" flag: invalid namespace: upstream-proxy, it must match "^[a-zA-Z_:][a-zA-Z0-9_:]*$"
See 'forwarder run --help' for usage.
exit status 1

@Choraden Choraden linked an issue Nov 10, 2023 that may be closed by this pull request
Copy link

what-the-diff bot commented Nov 10, 2023

PR Summary

  • Customization for Prometheus Namespace
    The ability to specify a custom Prometheus namespace for metrics has been introduced. This is a result of an update in the flag.go file that now accepts such specified values.

  • Addition of Namespace Validation
    A new function, ParsePrometheusNamespace, has been added to check the validity of the entered Prometheus namespace. This function, located in the config.go file, ensures the namespace adheres to the required format.

  • Inclusion of Namespace Validation in HTTP Server Configuration
    An additional layer of validation is added to the HTTP Server configuration, to further confirm that the Prometheus namespace is valid. If the namespace is found to be invalid during this check, an error will be returned promptly.

@mmatczuk
Copy link
Contributor

It feels like http_server: validate prometheus namespace goes beyond scope of http server validation.

I'd prefer to limit that to binding.

config.go Outdated
}

// https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
pattern := "^[a-zA-Z_:][a-zA-Z0-9_:]*$"
Copy link
Contributor

@mmatczuk mmatczuk Nov 13, 2023

Choose a reason for hiding this comment

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

As a global var we'd benefit from comptime validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's global now.

@Choraden
Copy link
Contributor Author

I limited the verification to binding only.

@mmatczuk mmatczuk merged commit 0505216 into main Nov 14, 2023
@mmatczuk mmatczuk deleted the hg/valid_prom_ns branch November 14, 2023 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FORWARDER_PROM_NAMESPACE must be validated
2 participants