Handle review feedback

README.md

@@ -72,6 +72,8 @@ This will create an OpenTelemetry Collector instance named `simplest`, exposing
 
 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.
 
+> 🚨 **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.
+
 > 🚨 **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.
 
 The Operator does examine the configuration file for a few purposes:

apis/v1beta1/config.go

@@ -434,25 +434,30 @@ const (
 
 // MetricsEndpoint attempts gets the host and port number from the host address without doing any validation regarding the
 // address itself.
-// It works even before env var expansion happens, when a simple `net.SplitHostPort` would fail because of  the extra colon
+// It works even before env var expansion happens, when a simple `net.SplitHostPort` would fail because of the extra colon
 // from the env var, i.e. the address looks like "${env:POD_IP}:4317", "${env:POD_IP}", or "${POD_IP}".
-// In cases which the port itself is a variable, i.e. "${env:POD_IP}:${env:PORT}", this returns an error.
-// It should work with IPv4 and IPv6 addresses.
+// In cases which the port itself is a variable, i.e. "${env:POD_IP}:${env:PORT}", this returns an error. This happens
+// because the port is used to generate Service objects and mappings.
 func (s *Service) MetricsEndpoint(logger logr.Logger) (string, int32, error) {
 	telemetry := s.GetTelemetry()
 	if telemetry == nil || telemetry.Metrics.Address == "" {
 		return defaultServiceHost, defaultServicePort, nil
 	}
 
-	isPortEnvVar := regexp.MustCompile(`:\${[env:]?.*}$`).MatchString(telemetry.Metrics.Address)
+	// The regex below matches on strings that end with a colon followed by the environment variable expansion syntax.
+	// So it should match on strings ending with: ":${env:POD_IP}" or ":${POD_IP}".
+	const portEnvVarRegex = `:\${[env:]?.*}$`
+	isPortEnvVar := regexp.MustCompile(portEnvVarRegex).MatchString(telemetry.Metrics.Address)
 	if isPortEnvVar {
 		errMsg := fmt.Sprintf("couldn't determine metrics port from configuration: %s",
 			telemetry.Metrics.Address)
 		logger.Info(errMsg)
 		return "", 0, fmt.Errorf(errMsg)
 	}
 
-	explicitPortMatches := regexp.MustCompile(`:(\d+$)`).FindStringSubmatch(telemetry.Metrics.Address)
+	// The regex below matches on strings that end with a colon followed by 1 or more numbers (representing the port).
+	const explicitPortRegex = `:(\d+$)`
+	explicitPortMatches := regexp.MustCompile(explicitPortRegex).FindStringSubmatch(telemetry.Metrics.Address)
 	if len(explicitPortMatches) <= 1 {
 		return telemetry.Metrics.Address, defaultServicePort, nil
 	}