Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…y-operator into 2947-updating-ds-sf-depl-mutation
  • Loading branch information
davidhaja committed Oct 16, 2024
2 parents 1d1ebb1 + b703b78 commit 2ae1acf
Show file tree
Hide file tree
Showing 46 changed files with 1,573 additions and 151 deletions.
18 changes: 18 additions & 0 deletions .chloggen/fix_validation-stabilizationWindowSeconds.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: collector-webhook

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Fixed validation of `stabilizationWindowSeconds` in autoscaler behaviour"

# One or more tracking issues related to the change
issues: [3345]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
The validation of `stabilizationWindowSeconds` in the `autoscaler.behaviour.scale[Up|Down]` incorrectly rejected 0 as an invalid value.
This has been fixed to ensure that the value is validated correctly (should be >=0 and <=3600) and the error messsage has been updated to reflect this.
34 changes: 34 additions & 0 deletions .chloggen/inst-tls.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: auto-instrumentation

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add support for specifying exporter TLS certificates in auto-instrumentation.

# One or more tracking issues related to the change
issues: [3338]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
Now Instrumentation CR supports specifying TLS certificates for exporter:
```yaml
spec:
exporter:
endpoint: https://otel-collector:4317
tls:
secretName: otel-tls-certs
configMapName: otel-ca-bundle
# otel-ca-bundle
ca_file: ca.crt
# present in otel-tls-certs
cert_file: tls.crt
# present in otel-tls-certs
key_file: tls.key
```
* Propagating secrets across namespaces can be done with https://github.com/EmberStack/kubernetes-reflector or https://github.com/zakkg3/ClusterSecret
* Restarting workloads on certificate renewal can be done with https://github.com/stakater/Reloader or https://github.com/wave-k8s/wave
21 changes: 21 additions & 0 deletions .chloggen/native_sidecar.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: collector

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add native sidecar injection behind a feature gate which is disabled by default.

# One or more tracking issues related to the change
issues: [2376]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
Native sidecars are supported since Kubernetes version `1.28` and are availabe by default since `1.29`.
To use native sidecars on Kubernetes v1.28 make sure the "SidecarContainers" feature gate on kubernetes is enabled.
If native sidecars are available, the operator can be advised to use them by adding adding
the `--feature-gates=operator.sidecarcontainers.native` to the Operator args.
In the future this may will become availabe as deployment mode on the Collector CR. See [#3356](https://github.com/open-telemetry/opentelemetry-operator/issues/3356)
3 changes: 3 additions & 0 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ jobs:
setup: "add-operator-arg OPERATOR_ARG='--feature-gates=operator.targetallocator.mtls' add-certmanager-permissions prepare-e2e"
- group: e2e-automatic-rbac
setup: "add-rbac-permissions-to-operator prepare-e2e"
- group: e2e-native-sidecar
setup: "add-operator-arg OPERATOR_ARG='--feature-gates=operator.sidecarcontainers.native' prepare-e2e"
kube-version: "1.29"
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@v4
Expand Down
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,13 @@ generate: controller-gen
e2e: chainsaw
$(CHAINSAW) test --test-dir ./tests/e2e

# e2e-native-sidecar
# NOTE: make sure the k8s featuregate "SidecarContainers" is set to true.
# NOTE: make sure the operator featuregate "operator.sidecarcontainers.native" is enabled.
.PHONY: e2e-native-sidecar
e2e-native-sidecar: chainsaw
$(CHAINSAW) test --test-dir ./tests/e2e-native-sidecar

# end-to-end-test for testing automatic RBAC creation
.PHONY: e2e-automatic-rbac
e2e-automatic-rbac: chainsaw
Expand Down
29 changes: 29 additions & 0 deletions apis/v1alpha1/instrumentation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,37 @@ type Resource struct {
// Exporter defines OTLP exporter configuration.
type Exporter struct {
// Endpoint is address of the collector with OTLP endpoint.
// If the endpoint defines https:// scheme TLS has to be specified.
// +optional
Endpoint string `json:"endpoint,omitempty"`

// TLS defines certificates for TLS.
// TLS needs to be enabled by specifying https:// scheme in the Endpoint.
TLS *TLS `json:"tls,omitempty"`
}

// TLS defines TLS configuration for exporter.
type TLS struct {
// SecretName defines secret name that will be used to configure TLS on the exporter.
// It is user responsibility to create the secret in the namespace of the workload.
// The secret must contain client certificate (Cert) and private key (Key).
// The CA certificate might be defined in the secret or in the config map.
SecretName string `json:"secretName,omitempty"`

// ConfigMapName defines configmap name with CA certificate. If it is not defined CA certificate will be
// used from the secret defined in SecretName.
ConfigMapName string `json:"configMapName,omitempty"`

// CA defines the key of certificate (e.g. ca.crt) in the configmap map, secret or absolute path to a certificate.
// The absolute path can be used when certificate is already present on the workload filesystem e.g.
// /var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt
CA string `json:"ca_file,omitempty"`
// Cert defines the key (e.g. tls.crt) of the client certificate in the secret or absolute path to a certificate.
// The absolute path can be used when certificate is already present on the workload filesystem.
Cert string `json:"cert_file,omitempty"`
// Key defines a key (e.g. tls.key) of the private key in the secret or absolute path to a certificate.
// The absolute path can be used when certificate is already present on the workload filesystem.
Key string `json:"key_file,omitempty"`
}

// Sampler defines sampling configuration.
Expand Down
22 changes: 22 additions & 0 deletions apis/v1alpha1/instrumentation_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,31 @@ func (w InstrumentationWebhook) validate(r *Instrumentation) (admission.Warnings
default:
return warnings, fmt.Errorf("spec.sampler.type is not valid: %s", r.Spec.Sampler.Type)
}

warnings = append(warnings, validateExporter(r.Spec.Exporter)...)

return warnings, nil
}

func validateExporter(exporter Exporter) []string {
var warnings []string
if exporter.TLS != nil {
tls := exporter.TLS
if tls.Key != "" && tls.Cert == "" || tls.Cert != "" && tls.Key == "" {
warnings = append(warnings, "both exporter.tls.key and exporter.tls.cert mut be set")
}

if !strings.HasPrefix(exporter.Endpoint, "https://") {
warnings = append(warnings, "exporter.tls is configured but exporter.endpoint is not enabling TLS with https://")
}
}
if strings.HasPrefix(exporter.Endpoint, "https://") && exporter.TLS == nil {
warnings = append(warnings, "exporter is using https:// but exporter.tls is unset")
}

return warnings
}

func validateJaegerRemoteSamplerArgument(argument string) error {
parts := strings.Split(argument, ",")

Expand Down
88 changes: 88 additions & 0 deletions apis/v1alpha1/instrumentation_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,94 @@ func TestInstrumentationValidatingWebhook(t *testing.T) {
},
},
},
{
name: "exporter: tls cert set but missing key",
inst: Instrumentation{
Spec: InstrumentationSpec{
Sampler: Sampler{
Type: ParentBasedTraceIDRatio,
Argument: "0.99",
},
Exporter: Exporter{
Endpoint: "https://collector:4317",
TLS: &TLS{
Cert: "cert",
},
},
},
},
warnings: []string{"both exporter.tls.key and exporter.tls.cert mut be set"},
},
{
name: "exporter: tls key set but missing cert",
inst: Instrumentation{
Spec: InstrumentationSpec{
Sampler: Sampler{
Type: ParentBasedTraceIDRatio,
Argument: "0.99",
},
Exporter: Exporter{
Endpoint: "https://collector:4317",
TLS: &TLS{
Key: "key",
},
},
},
},
warnings: []string{"both exporter.tls.key and exporter.tls.cert mut be set"},
},
{
name: "exporter: tls set but using http://",
inst: Instrumentation{
Spec: InstrumentationSpec{
Sampler: Sampler{
Type: ParentBasedTraceIDRatio,
Argument: "0.99",
},
Exporter: Exporter{
Endpoint: "http://collector:4317",
TLS: &TLS{
Key: "key",
Cert: "cert",
},
},
},
},
warnings: []string{"exporter.tls is configured but exporter.endpoint is not enabling TLS with https://"},
},
{
name: "exporter: exporter using http://, but the tls is nil",
inst: Instrumentation{
Spec: InstrumentationSpec{
Sampler: Sampler{
Type: ParentBasedTraceIDRatio,
Argument: "0.99",
},
Exporter: Exporter{
Endpoint: "https://collector:4317",
},
},
},
warnings: []string{"exporter is using https:// but exporter.tls is unset"},
},
{
name: "exporter no warning set",
inst: Instrumentation{
Spec: InstrumentationSpec{
Sampler: Sampler{
Type: ParentBasedTraceIDRatio,
Argument: "0.99",
},
Exporter: Exporter{
Endpoint: "https://collector:4317",
TLS: &TLS{
Key: "key",
Cert: "cert",
},
},
},
},
},
}

for _, test := range tests {
Expand Down
22 changes: 21 additions & 1 deletion apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions apis/v1beta1/collector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,13 @@ func ValidatePorts(ports []PortsSpec) error {
func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error {
if autoscaler.Behavior != nil {
if autoscaler.Behavior.ScaleDown != nil && autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds != nil &&
*autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds < int32(1) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleDown should be one or more")
(*autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds < int32(0) || *autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds > 3600) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleDown.stabilizationWindowSeconds should be >=0 and <=3600")
}

if autoscaler.Behavior.ScaleUp != nil && autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds != nil &&
*autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds < int32(1) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleUp should be one or more")
(*autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds < int32(0) || *autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds > 3600) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleUp.stabilizationWindowSeconds should be >=0 and <=3600")
}
}
if autoscaler.TargetCPUUtilization != nil && *autoscaler.TargetCPUUtilization < int32(1) {
Expand Down
Loading

0 comments on commit 2ae1acf

Please sign in to comment.