Skip to content

Commit a6744fd

Browse files
committed
Handle review feedback
1 parent a4ef4f0 commit a6744fd

File tree

2 files changed

+12
-5
lines changed

2 files changed

+12
-5
lines changed

README.md

+2
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ This will create an OpenTelemetry Collector instance named `simplest`, exposing
7272

7373
The `config` node holds the `YAML` that should be passed down as-is to the underlying OpenTelemetry Collector instances. Refer to the [OpenTelemetry Collector](https://github.com/open-telemetry/opentelemetry-collector) documentation for a reference of the possible entries.
7474

75+
> 🚨 **NOTE:** At this point, the Operator does _not_ validate the whole contents of the configuration file: if the configuration is invalid, the instance might still be created but the underlying OpenTelemetry Collector might crash.
76+
7577
> 🚨 **Note:** For private GKE clusters, you will need to either add a firewall rule that allows master nodes access to port `9443/tcp` on worker nodes, or change the existing rule that allows access to port `80/tcp`, `443/tcp` and `10254/tcp` to also allow access to port `9443/tcp`. More information can be found in the [Official GCP Documentation](https://cloud.google.com/load-balancing/docs/tcp/setting-up-tcp#config-hc-firewall). See the [GKE documentation](https://cloud.google.com/kubernetes-engine/docs/how-to/private-clusters#add_firewall_rules) on adding rules and the [Kubernetes issue](https://github.com/kubernetes/kubernetes/issues/79739) for more detail.
7678
7779
The Operator does examine the configuration file for a few purposes:

apis/v1beta1/config.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -434,25 +434,30 @@ const (
434434

435435
// MetricsEndpoint attempts gets the host and port number from the host address without doing any validation regarding the
436436
// address itself.
437-
// It works even before env var expansion happens, when a simple `net.SplitHostPort` would fail because of the extra colon
437+
// It works even before env var expansion happens, when a simple `net.SplitHostPort` would fail because of the extra colon
438438
// from the env var, i.e. the address looks like "${env:POD_IP}:4317", "${env:POD_IP}", or "${POD_IP}".
439-
// In cases which the port itself is a variable, i.e. "${env:POD_IP}:${env:PORT}", this returns an error.
440-
// It should work with IPv4 and IPv6 addresses.
439+
// In cases which the port itself is a variable, i.e. "${env:POD_IP}:${env:PORT}", this returns an error. This happens
440+
// because the port is used to generate Service objects and mappings.
441441
func (s *Service) MetricsEndpoint(logger logr.Logger) (string, int32, error) {
442442
telemetry := s.GetTelemetry()
443443
if telemetry == nil || telemetry.Metrics.Address == "" {
444444
return defaultServiceHost, defaultServicePort, nil
445445
}
446446

447-
isPortEnvVar := regexp.MustCompile(`:\${[env:]?.*}$`).MatchString(telemetry.Metrics.Address)
447+
// The regex below matches on strings that end with a colon followed by the environment variable expansion syntax.
448+
// So it should match on strings ending with: ":${env:POD_IP}" or ":${POD_IP}".
449+
const portEnvVarRegex = `:\${[env:]?.*}$`
450+
isPortEnvVar := regexp.MustCompile(portEnvVarRegex).MatchString(telemetry.Metrics.Address)
448451
if isPortEnvVar {
449452
errMsg := fmt.Sprintf("couldn't determine metrics port from configuration: %s",
450453
telemetry.Metrics.Address)
451454
logger.Info(errMsg)
452455
return "", 0, fmt.Errorf(errMsg)
453456
}
454457

455-
explicitPortMatches := regexp.MustCompile(`:(\d+$)`).FindStringSubmatch(telemetry.Metrics.Address)
458+
// The regex below matches on strings that end with a colon followed by 1 or more numbers (representing the port).
459+
const explicitPortRegex = `:(\d+$)`
460+
explicitPortMatches := regexp.MustCompile(explicitPortRegex).FindStringSubmatch(telemetry.Metrics.Address)
456461
if len(explicitPortMatches) <= 1 {
457462
return telemetry.Metrics.Address, defaultServicePort, nil
458463
}

0 commit comments

Comments
 (0)