diff --git a/.chloggen/enhanced-webhook.yaml b/.chloggen/enhanced-webhook.yaml new file mode 100644 index 0000000000..ddacb9ee1a --- /dev/null +++ b/.chloggen/enhanced-webhook.yaml @@ -0,0 +1,16 @@ +# 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. operator, target allocator, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Added reconciliation errors for webhook events. The webhooks run the manifest generators to check for any errors. + +# One or more tracking issues related to the change +issues: [2399] + +# (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: \ No newline at end of file diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index c44f9f48ab..4e783a01df 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -47,6 +47,7 @@ type CollectorWebhook struct { scheme *runtime.Scheme reviewer *rbac.Reviewer metrics *Metrics + bv BuildValidator } func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error { @@ -108,14 +109,17 @@ func (c CollectorWebhook) ValidateCreate(ctx context.Context, obj runtime.Object return nil, fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj) } - warnings, err := c.validate(ctx, otelcol) + warnings, err := c.Validate(ctx, otelcol) if err != nil { return warnings, err } if c.metrics != nil { c.metrics.create(ctx, otelcol) } - + if c.bv != nil { + newWarnings := c.bv(*otelcol) + warnings = append(warnings, newWarnings...) + } return warnings, nil } @@ -133,7 +137,7 @@ func (c CollectorWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj run if otelcolOld.Spec.Mode != otelcol.Spec.Mode { return admission.Warnings{}, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support modification", otelcolOld.Spec.Mode) } - warnings, err := c.validate(ctx, otelcol) + warnings, err := c.Validate(ctx, otelcol) if err != nil { return warnings, err } @@ -142,6 +146,10 @@ func (c CollectorWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj run c.metrics.update(ctx, otelcolOld, otelcol) } + if c.bv != nil { + newWarnings := c.bv(*otelcol) + warnings = append(warnings, newWarnings...) + } return warnings, nil } @@ -151,7 +159,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, obj runtime.Object return nil, fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj) } - warnings, err := c.validate(ctx, otelcol) + warnings, err := c.Validate(ctx, otelcol) if err != nil { return warnings, err } @@ -163,7 +171,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, obj runtime.Object return warnings, nil } -func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollector) (admission.Warnings, error) { +func (c CollectorWebhook) Validate(ctx context.Context, r *OpenTelemetryCollector) (admission.Warnings, error) { warnings := admission.Warnings{} nullObjects := r.Spec.Config.nullObjects() @@ -404,14 +412,30 @@ func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error { return nil } -func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics) error { - cvw := &CollectorWebhook{ - reviewer: reviewer, - logger: mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"), - scheme: mgr.GetScheme(), +// BuildValidator enables running the manifest generators for the collector reconciler +// +kubebuilder:object:generate=false +type BuildValidator func(c OpenTelemetryCollector) admission.Warnings + +func NewCollectorWebhook( + logger logr.Logger, + scheme *runtime.Scheme, + cfg config.Config, + reviewer *rbac.Reviewer, + metrics *Metrics, + bv BuildValidator, +) *CollectorWebhook { + return &CollectorWebhook{ + logger: logger, + scheme: scheme, cfg: cfg, + reviewer: reviewer, metrics: metrics, + bv: bv, } +} + +func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics, bv BuildValidator) error { + cvw := NewCollectorWebhook(mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"), mgr.GetScheme(), cfg, reviewer, metrics, bv) return ctrl.NewWebhookManagedBy(mgr). For(&OpenTelemetryCollector{}). WithValidator(cvw). diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index 112a782488..64ffff48ea 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/apis/v1beta1/collector_webhook_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package v1beta1 +package v1beta1_test import ( "context" @@ -35,8 +35,12 @@ import ( "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/scheme" kubeTesting "k8s.io/client-go/testing" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + collectorManifests "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" ) @@ -46,23 +50,24 @@ var ( func TestValidate(t *testing.T) { tests := []struct { - name string - collector OpenTelemetryCollector - warnings []string - err string + name string + collector v1beta1.OpenTelemetryCollector + warnings []string + err string + shouldFailSar bool }{ { name: "Test ", - collector: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Config: Config{ - Processors: &AnyConfig{ + collector: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Config: v1beta1.Config{ + Processors: &v1beta1.AnyConfig{ Object: map[string]interface{}{ "batch": nil, "foo": nil, }, }, - Extensions: &AnyConfig{ + Extensions: &v1beta1.AnyConfig{ Object: map[string]interface{}{ "foo": nil, }, @@ -76,11 +81,42 @@ func TestValidate(t *testing.T) { }, }, } + + bv := func(collector v1beta1.OpenTelemetryCollector) admission.Warnings { + var warnings admission.Warnings + cfg := config.New( + config.WithCollectorImage("default-collector"), + config.WithTargetAllocatorImage("default-ta-allocator"), + ) + params := manifests.Params{ + Log: logr.Discard(), + Config: cfg, + OtelCol: collector, + } + _, err := collectorManifests.Build(params) + if err != nil { + warnings = append(warnings, err.Error()) + return warnings + } + return nil + } + for _, tt := range tests { - webhook := CollectorWebhook{} + test := tt + webhook := v1beta1.NewCollectorWebhook( + logr.Discard(), + testScheme, + config.New( + config.WithCollectorImage("collector:v0.0.0"), + config.WithTargetAllocatorImage("ta:v0.0.0"), + ), + getReviewer(test.shouldFailSar), + nil, + bv, + ) t.Run(tt.name, func(t *testing.T) { tt := tt - warnings, err := webhook.validate(context.Background(), &tt.collector) + warnings, err := webhook.Validate(context.Background(), &tt.collector) if tt.err == "" { require.NoError(t, err) } else { @@ -96,110 +132,111 @@ func TestCollectorDefaultingWebhook(t *testing.T) { five := int32(5) defaultCPUTarget := int32(90) - if err := AddToScheme(testScheme); err != nil { + if err := v1beta1.AddToScheme(testScheme); err != nil { fmt.Printf("failed to register scheme: %v", err) os.Exit(1) } tests := []struct { - name string - otelcol OpenTelemetryCollector - expected OpenTelemetryCollector + name string + otelcol v1beta1.OpenTelemetryCollector + expected v1beta1.OpenTelemetryCollector + shouldFailSar bool }{ { name: "all fields default", - otelcol: OpenTelemetryCollector{}, - expected: OpenTelemetryCollector{ + otelcol: v1beta1.OpenTelemetryCollector{}, + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, Replicas: &one, }, - Mode: ModeDeployment, - UpgradeStrategy: UpgradeStrategyAutomatic, + Mode: v1beta1.ModeDeployment, + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, }, }, }, { name: "provided values in spec", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, UpgradeStrategy: "adhoc", - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Replicas: &five, }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, UpgradeStrategy: "adhoc", - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &five, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, }, }, }, }, { name: "doesn't override unmanaged", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, UpgradeStrategy: "adhoc", - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Replicas: &five, - ManagementState: ManagementStateUnmanaged, + ManagementState: v1beta1.ManagementStateUnmanaged, }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, UpgradeStrategy: "adhoc", - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &five, - ManagementState: ManagementStateUnmanaged, + ManagementState: v1beta1.ManagementStateUnmanaged, }, }, }, }, { name: "Setting Autoscaler MaxReplicas", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &five, MinReplicas: &one, }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - UpgradeStrategy: UpgradeStrategyAutomatic, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &one, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, }, - Autoscaler: &AutoscalerSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ TargetCPUUtilization: &defaultCPUTarget, MaxReplicas: &five, MinReplicas: &one, @@ -209,42 +246,42 @@ func TestCollectorDefaultingWebhook(t *testing.T) { }, { name: "Missing route termination", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - Ingress: Ingress{ - Type: IngressTypeRoute, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + Ingress: v1beta1.Ingress{ + Type: v1beta1.IngressTypeRoute, }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, Replicas: &one, }, - Ingress: Ingress{ - Type: IngressTypeRoute, - Route: OpenShiftRoute{ - Termination: TLSRouteTerminationTypeEdge, + Ingress: v1beta1.Ingress{ + Type: v1beta1.IngressTypeRoute, + Route: v1beta1.OpenShiftRoute{ + Termination: v1beta1.TLSRouteTerminationTypeEdge, }, }, - UpgradeStrategy: UpgradeStrategyAutomatic, + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, }, }, }, { name: "Defined PDB for collector", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ - PodDisruptionBudget: &PodDisruptionBudgetSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.String, StrVal: "10%", @@ -253,36 +290,36 @@ func TestCollectorDefaultingWebhook(t *testing.T) { }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &one, - ManagementState: ManagementStateManaged, - PodDisruptionBudget: &PodDisruptionBudgetSpec{ + ManagementState: v1beta1.ManagementStateManaged, + PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.String, StrVal: "10%", }, }, }, - UpgradeStrategy: UpgradeStrategyAutomatic, + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, }, }, }, { name: "Defined PDB for target allocator", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, - AllocationStrategy: TargetAllocatorAllocationStrategyConsistentHashing, - PodDisruptionBudget: &PodDisruptionBudgetSpec{ + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyConsistentHashing, + PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.String, StrVal: "10%", @@ -291,23 +328,23 @@ func TestCollectorDefaultingWebhook(t *testing.T) { }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &one, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, }, - UpgradeStrategy: UpgradeStrategyAutomatic, - TargetAllocator: TargetAllocatorEmbedded{ + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, Replicas: &one, - AllocationStrategy: TargetAllocatorAllocationStrategyConsistentHashing, - PodDisruptionBudget: &PodDisruptionBudgetSpec{ + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyConsistentHashing, + PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.String, StrVal: "10%", @@ -319,13 +356,13 @@ func TestCollectorDefaultingWebhook(t *testing.T) { }, { name: "Defined PDB for target allocator per-node", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, - AllocationStrategy: TargetAllocatorAllocationStrategyPerNode, - PodDisruptionBudget: &PodDisruptionBudgetSpec{ + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyPerNode, + PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.String, StrVal: "10%", @@ -334,23 +371,23 @@ func TestCollectorDefaultingWebhook(t *testing.T) { }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &one, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, }, - UpgradeStrategy: UpgradeStrategyAutomatic, - TargetAllocator: TargetAllocatorEmbedded{ + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, Replicas: &one, - AllocationStrategy: TargetAllocatorAllocationStrategyPerNode, - PodDisruptionBudget: &PodDisruptionBudgetSpec{ + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyPerNode, + PodDisruptionBudget: &v1beta1.PodDisruptionBudgetSpec{ MinAvailable: &intstr.IntOrString{ Type: intstr.String, StrVal: "10%", @@ -362,80 +399,102 @@ func TestCollectorDefaultingWebhook(t *testing.T) { }, { name: "Undefined PDB for target allocator and consistent-hashing strategy", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, Replicas: &one, - AllocationStrategy: TargetAllocatorAllocationStrategyConsistentHashing, + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyConsistentHashing, }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &one, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, }, - UpgradeStrategy: UpgradeStrategyAutomatic, - TargetAllocator: TargetAllocatorEmbedded{ + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, Replicas: &one, - AllocationStrategy: TargetAllocatorAllocationStrategyConsistentHashing, + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyConsistentHashing, }, }, }, }, { name: "Undefined PDB for target allocator and not consistent-hashing strategy", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, - AllocationStrategy: TargetAllocatorAllocationStrategyLeastWeighted, + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyLeastWeighted, }, }, }, - expected: OpenTelemetryCollector{ + expected: v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Args: map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"}, Replicas: &one, - ManagementState: ManagementStateManaged, + ManagementState: v1beta1.ManagementStateManaged, }, - UpgradeStrategy: UpgradeStrategyAutomatic, - TargetAllocator: TargetAllocatorEmbedded{ + UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, Replicas: &one, - AllocationStrategy: TargetAllocatorAllocationStrategyLeastWeighted, + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyLeastWeighted, }, }, }, }, } + bv := func(collector v1beta1.OpenTelemetryCollector) admission.Warnings { + var warnings admission.Warnings + cfg := config.New( + config.WithCollectorImage("default-collector"), + config.WithTargetAllocatorImage("default-ta-allocator"), + ) + params := manifests.Params{ + Log: logr.Discard(), + Config: cfg, + OtelCol: collector, + } + _, err := collectorManifests.Build(params) + if err != nil { + warnings = append(warnings, err.Error()) + return warnings + } + return nil + } + for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - cvw := &CollectorWebhook{ - logger: logr.Discard(), - scheme: testScheme, - cfg: config.New( + cvw := v1beta1.NewCollectorWebhook( + logr.Discard(), + testScheme, + config.New( config.WithCollectorImage("collector:v0.0.0"), config.WithTargetAllocatorImage("ta:v0.0.0"), ), - } + getReviewer(test.shouldFailSar), + nil, + bv, + ) ctx := context.Background() err := cvw.Default(ctx, &test.otelcol) assert.NoError(t, err) @@ -468,29 +527,29 @@ func TestOTELColValidatingWebhook(t *testing.T) { three := int32(3) five := int32(5) - cfg := Config{} + cfg := v1beta1.Config{} err := yaml.Unmarshal([]byte(cfgYaml), &cfg) require.NoError(t, err) tests := []struct { //nolint:govet name string - otelcol OpenTelemetryCollector + otelcol v1beta1.OpenTelemetryCollector expectedErr string expectedWarnings []string shouldFailSar bool }{ { name: "valid empty spec", - otelcol: OpenTelemetryCollector{}, + otelcol: v1beta1.OpenTelemetryCollector{}, }, { name: "valid full spec", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeStatefulSet, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeStatefulSet, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Replicas: &three, - Ports: []PortsSpec{ + Ports: []v1beta1.PortsSpec{ { ServicePort: v1.ServicePort{ Name: "port1", @@ -506,7 +565,7 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, }, }, - Autoscaler: &AutoscalerSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MinReplicas: &one, MaxReplicas: &five, Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ @@ -520,7 +579,7 @@ func TestOTELColValidatingWebhook(t *testing.T) { TargetCPUUtilization: &five, }, UpgradeStrategy: "adhoc", - TargetAllocator: TargetAllocatorEmbedded{ + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, }, Config: cfg, @@ -530,12 +589,12 @@ func TestOTELColValidatingWebhook(t *testing.T) { { name: "prom CR admissions warning", shouldFailSar: true, // force failure - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeStatefulSet, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeStatefulSet, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Replicas: &three, - Ports: []PortsSpec{ + Ports: []v1beta1.PortsSpec{ { ServicePort: v1.ServicePort{ Name: "port1", @@ -551,7 +610,7 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, }, }, - Autoscaler: &AutoscalerSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MinReplicas: &one, MaxReplicas: &five, Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ @@ -565,9 +624,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { TargetCPUUtilization: &five, }, UpgradeStrategy: "adhoc", - TargetAllocator: TargetAllocatorEmbedded{ + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, - PrometheusCR: TargetAllocatorPrometheusCR{Enabled: true}, + PrometheusCR: v1beta1.TargetAllocatorPrometheusCR{Enabled: true}, }, Config: cfg, }, @@ -590,13 +649,13 @@ func TestOTELColValidatingWebhook(t *testing.T) { { name: "prom CR no admissions warning", shouldFailSar: false, // force SAR okay - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeStatefulSet, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeStatefulSet, UpgradeStrategy: "adhoc", - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Replicas: &three, - Ports: []PortsSpec{ + Ports: []v1beta1.PortsSpec{ { ServicePort: v1.ServicePort{ Name: "port1", @@ -612,7 +671,7 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, }, }, - Autoscaler: &AutoscalerSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ ScaleDown: &autoscalingv2.HPAScalingRules{ StabilizationWindowSeconds: &three, @@ -623,9 +682,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, TargetCPUUtilization: &five, }, - TargetAllocator: TargetAllocatorEmbedded{ + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, - PrometheusCR: TargetAllocatorPrometheusCR{Enabled: true}, + PrometheusCR: v1beta1.TargetAllocatorPrometheusCR{Enabled: true}, }, Config: cfg, }, @@ -633,10 +692,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid mode with volume claim templates", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - StatefulSetCommonFields: StatefulSetCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + StatefulSetCommonFields: v1beta1.StatefulSetCommonFields{ VolumeClaimTemplates: []v1.PersistentVolumeClaim{{}, {}}, }, }, @@ -645,10 +704,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid mode with tolerations", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Tolerations: []v1.Toleration{{}, {}}, }, }, @@ -657,10 +716,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid mode with target allocator", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, }, }, @@ -669,10 +728,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid target allocator config", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeStatefulSet, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeStatefulSet, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, }, }, @@ -681,12 +740,12 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid target allocation strategy", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDaemonSet, - TargetAllocator: TargetAllocatorEmbedded{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDaemonSet, + TargetAllocator: v1beta1.TargetAllocatorEmbedded{ Enabled: true, - AllocationStrategy: TargetAllocatorAllocationStrategyLeastWeighted, + AllocationStrategy: v1beta1.TargetAllocatorAllocationStrategyLeastWeighted, }, }, }, @@ -694,10 +753,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid port name", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ - Ports: []PortsSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Ports: []v1beta1.PortsSpec{ { ServicePort: v1.ServicePort{ // this port name contains a non alphanumeric character, which is invalid. @@ -714,10 +773,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid port name, too long", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ - Ports: []PortsSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Ports: []v1beta1.PortsSpec{ { ServicePort: v1.ServicePort{ Name: "aaaabbbbccccdddd", // len: 16, too long @@ -732,10 +791,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid port num", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ - Ports: []PortsSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Ports: []v1beta1.PortsSpec{ { ServicePort: v1.ServicePort{ Name: "aaaabbbbccccddd", // len: 15 @@ -750,9 +809,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid max replicas", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &zero, }, }, @@ -761,12 +820,12 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid replicas, greater than max", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Replicas: &five, }, - Autoscaler: &AutoscalerSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, }, }, @@ -775,9 +834,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid min replicas, greater than max", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, MinReplicas: &five, }, @@ -787,9 +846,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid min replicas, lesser than 1", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, MinReplicas: &zero, }, @@ -799,9 +858,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid autoscaler scale down", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ ScaleDown: &autoscalingv2.HPAScalingRules{ @@ -815,9 +874,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid autoscaler scale up", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ ScaleUp: &autoscalingv2.HPAScalingRules{ @@ -831,9 +890,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid autoscaler target cpu utilization", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, TargetCPUUtilization: &zero, }, @@ -843,9 +902,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid autoscaler target memory utilization", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, TargetMemoryUtilization: &zero, }, @@ -855,9 +914,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "autoscaler minReplicas is less than maxReplicas", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &one, MinReplicas: &five, }, @@ -867,11 +926,11 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid autoscaler metric type", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, - Metrics: []MetricSpec{ + Metrics: []v1beta1.MetricSpec{ { Type: autoscalingv2.ResourceMetricSourceType, }, @@ -883,11 +942,11 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid pod metric average value", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, - Metrics: []MetricSpec{ + Metrics: []v1beta1.MetricSpec{ { Type: autoscalingv2.PodsMetricSourceType, Pods: &autoscalingv2.PodsMetricSource{ @@ -908,11 +967,11 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "utilization target is not valid with pod metrics", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Autoscaler: &AutoscalerSpec{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, - Metrics: []MetricSpec{ + Metrics: []v1beta1.MetricSpec{ { Type: autoscalingv2.PodsMetricSourceType, Pods: &autoscalingv2.PodsMetricSource{ @@ -932,25 +991,23 @@ func TestOTELColValidatingWebhook(t *testing.T) { expectedErr: "the OpenTelemetry Spec autoscale configuration is incorrect, invalid pods target type", }, { - name: "invalid deployment mode incompabible with ingress settings", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - Ingress: Ingress{ - Type: IngressTypeIngress, + name: "invalid deployment mode incompatible with ingress settings", + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + Ingress: v1beta1.Ingress{ + Type: v1beta1.IngressTypeIngress, }, }, }, - expectedErr: fmt.Sprintf("Ingress can only be used in combination with the modes: %s, %s, %s", - ModeDeployment, ModeDaemonSet, ModeStatefulSet, - ), + expectedErr: fmt.Sprintf("Ingress can only be used in combination with the modes: %s, %s, %s", v1beta1.ModeDeployment, v1beta1.ModeDaemonSet, v1beta1.ModeStatefulSet), }, { name: "invalid mode with priorityClassName", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ PriorityClassName: "test-class", }, }, @@ -959,10 +1016,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid mode with affinity", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ Affinity: &v1.Affinity{ NodeAffinity: &v1.NodeAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ @@ -987,9 +1044,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid InitialDelaySeconds", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - LivenessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + LivenessProbe: &v1beta1.Probe{ InitialDelaySeconds: &minusOne, }, }, @@ -998,9 +1055,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid InitialDelaySeconds readiness", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - ReadinessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + ReadinessProbe: &v1beta1.Probe{ InitialDelaySeconds: &minusOne, }, }, @@ -1009,9 +1066,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid PeriodSeconds", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - LivenessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + LivenessProbe: &v1beta1.Probe{ PeriodSeconds: &zero, }, }, @@ -1020,9 +1077,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid PeriodSeconds readiness", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - ReadinessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + ReadinessProbe: &v1beta1.Probe{ PeriodSeconds: &zero, }, }, @@ -1031,9 +1088,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid TimeoutSeconds", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - LivenessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + LivenessProbe: &v1beta1.Probe{ TimeoutSeconds: &zero, }, }, @@ -1042,9 +1099,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid TimeoutSeconds readiness", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - ReadinessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + ReadinessProbe: &v1beta1.Probe{ TimeoutSeconds: &zero, }, }, @@ -1053,9 +1110,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid SuccessThreshold", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - LivenessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + LivenessProbe: &v1beta1.Probe{ SuccessThreshold: &zero, }, }, @@ -1064,9 +1121,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid SuccessThreshold readiness", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - ReadinessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + ReadinessProbe: &v1beta1.Probe{ SuccessThreshold: &zero, }, }, @@ -1075,9 +1132,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid FailureThreshold", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - LivenessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + LivenessProbe: &v1beta1.Probe{ FailureThreshold: &zero, }, }, @@ -1086,9 +1143,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid FailureThreshold readiness", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - ReadinessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + ReadinessProbe: &v1beta1.Probe{ FailureThreshold: &zero, }, }, @@ -1097,9 +1154,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid TerminationGracePeriodSeconds", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - LivenessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + LivenessProbe: &v1beta1.Probe{ TerminationGracePeriodSeconds: &zero64, }, }, @@ -1108,9 +1165,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid TerminationGracePeriodSeconds readiness", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - ReadinessProbe: &Probe{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + ReadinessProbe: &v1beta1.Probe{ TerminationGracePeriodSeconds: &zero64, }, }, @@ -1119,10 +1176,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid AdditionalContainers", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - OpenTelemetryCommonFields: OpenTelemetryCommonFields{ + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ AdditionalContainers: []v1.Container{ { Name: "test", @@ -1135,10 +1192,10 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "missing ingress hostname for subdomain ruleType", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Ingress: Ingress{ - RuleType: IngressRuleTypeSubdomain, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Ingress: v1beta1.Ingress{ + RuleType: v1beta1.IngressRuleTypeSubdomain, }, }, }, @@ -1146,9 +1203,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid updateStrategy for Deployment mode", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeDeployment, DaemonSetUpdateStrategy: appsv1.DaemonSetUpdateStrategy{ Type: "RollingUpdate", RollingUpdate: &appsv1.RollingUpdateDaemonSet{ @@ -1162,9 +1219,9 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, { name: "invalid updateStrategy for Statefulset mode", - otelcol: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeStatefulSet, + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeStatefulSet, DeploymentUpdateStrategy: appsv1.DeploymentStrategy{ Type: "RollingUpdate", RollingUpdate: &appsv1.RollingUpdateDeployment{ @@ -1176,20 +1233,59 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Collector mode is set to statefulset, which does not support the attribute 'deploymentUpdateStrategy'", }, + { + name: "missing port for ingress type", + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Ports: []v1beta1.PortsSpec{ + { + ServicePort: v1.ServicePort{}, + }, + }, + }, + Ingress: v1beta1.Ingress{ + Type: v1beta1.IngressTypeIngress, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect", + }, + } + + bv := func(collector v1beta1.OpenTelemetryCollector) admission.Warnings { + var warnings admission.Warnings + cfg := config.New( + config.WithCollectorImage("default-collector"), + config.WithTargetAllocatorImage("default-ta-allocator"), + ) + params := manifests.Params{ + Log: logr.Discard(), + Config: cfg, + OtelCol: collector, + } + _, err := collectorManifests.Build(params) + if err != nil { + warnings = append(warnings, err.Error()) + return warnings + } + return nil } for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - cvw := &CollectorWebhook{ - logger: logr.Discard(), - scheme: testScheme, - cfg: config.New( + cvw := v1beta1.NewCollectorWebhook( + logr.Discard(), + testScheme, + config.New( config.WithCollectorImage("collector:v0.0.0"), config.WithTargetAllocatorImage("ta:v0.0.0"), ), - reviewer: getReviewer(test.shouldFailSar), - } + getReviewer(test.shouldFailSar), + nil, + bv, + ) ctx := context.Background() warnings, err := cvw.ValidateCreate(ctx, &test.otelcol) if test.expectedErr == "" { @@ -1206,35 +1302,57 @@ func TestOTELColValidatingWebhook(t *testing.T) { func TestOTELColValidateUpdateWebhook(t *testing.T) { tests := []struct { //nolint:govet name string - otelcolOld OpenTelemetryCollector - otelcolNew OpenTelemetryCollector + otelcolOld v1beta1.OpenTelemetryCollector + otelcolNew v1beta1.OpenTelemetryCollector expectedErr string expectedWarnings []string shouldFailSar bool }{ { name: "mode should not be changed", - otelcolOld: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{Mode: ModeStatefulSet}, + otelcolOld: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{Mode: v1beta1.ModeStatefulSet}, }, - otelcolNew: OpenTelemetryCollector{ - Spec: OpenTelemetryCollectorSpec{Mode: ModeDeployment}, + otelcolNew: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{Mode: v1beta1.ModeDeployment}, }, expectedErr: "which does not support modification", }, } + + bv := func(collector v1beta1.OpenTelemetryCollector) admission.Warnings { + var warnings admission.Warnings + cfg := config.New( + config.WithCollectorImage("default-collector"), + config.WithTargetAllocatorImage("default-ta-allocator"), + ) + params := manifests.Params{ + Log: logr.Discard(), + Config: cfg, + OtelCol: collector, + } + _, err := collectorManifests.Build(params) + if err != nil { + warnings = append(warnings, err.Error()) + return warnings + } + return nil + } + for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - cvw := &CollectorWebhook{ - logger: logr.Discard(), - scheme: testScheme, - cfg: config.New( + cvw := v1beta1.NewCollectorWebhook( + logr.Discard(), + testScheme, + config.New( config.WithCollectorImage("collector:v0.0.0"), config.WithTargetAllocatorImage("ta:v0.0.0"), ), - reviewer: getReviewer(test.shouldFailSar), - } + getReviewer(test.shouldFailSar), + nil, + bv, + ) ctx := context.Background() warnings, err := cvw.ValidateUpdate(ctx, &test.otelcolOld, &test.otelcolNew) if test.expectedErr == "" { diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 372a75ae43..5c5f0b84cb 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,3 +1,2 @@ resources: - manager.yaml - diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index be370ba795..8c616700a6 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -168,7 +168,7 @@ func (r *OpenTelemetryCollectorReconciler) getConfigMapsToRemove(configVersionsT return ownedConfigMaps } -func (r *OpenTelemetryCollectorReconciler) getParams(instance v1beta1.OpenTelemetryCollector) (manifests.Params, error) { +func (r *OpenTelemetryCollectorReconciler) GetParams(instance v1beta1.OpenTelemetryCollector) (manifests.Params, error) { p := manifests.Params{ Config: r.config, Client: r.Client, @@ -229,7 +229,7 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct return ctrl.Result{}, client.IgnoreNotFound(err) } - params, err := r.getParams(instance) + params, err := r.GetParams(instance) if err != nil { log.Error(err, "Failed to create manifest.Params") return ctrl.Result{}, err diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 0836809719..3e584f8f8a 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -178,7 +178,7 @@ func TestMain(m *testing.M) { } reviewer := rbac.NewReviewer(clientset) - if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil); err != nil { + if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil); err != nil { fmt.Printf("failed to SetupWebhookWithManager: %v", err) os.Exit(1) } diff --git a/internal/webhook/podmutation/webhookhandler_suite_test.go b/internal/webhook/podmutation/webhookhandler_suite_test.go index 1336cab0e8..8448762f5d 100644 --- a/internal/webhook/podmutation/webhookhandler_suite_test.go +++ b/internal/webhook/podmutation/webhookhandler_suite_test.go @@ -105,7 +105,7 @@ func TestMain(m *testing.M) { } reviewer := rbac.NewReviewer(clientset) - if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil); err != nil { + if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil); err != nil { fmt.Printf("failed to SetupWebhookWithManager: %v", err) os.Exit(1) } diff --git a/main.go b/main.go index fbfca13ffe..25913cd28f 100644 --- a/main.go +++ b/main.go @@ -53,6 +53,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" "github.com/open-telemetry/opentelemetry-operator/internal/config" + collectorManifests "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" openshiftDashboards "github.com/open-telemetry/opentelemetry-operator/internal/openshift/dashboards" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -365,13 +366,15 @@ func main() { os.Exit(1) } - if err = controllers.NewReconciler(controllers.Params{ + collectorReconciler := controllers.NewReconciler(controllers.Params{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("OpenTelemetryCollector"), Scheme: mgr.GetScheme(), Config: cfg, Recorder: mgr.GetEventRecorderFor("opentelemetry-operator"), - }).SetupWithManager(mgr); err != nil { + }) + + if err = collectorReconciler.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "OpenTelemetryCollector") os.Exit(1) } @@ -403,7 +406,22 @@ func main() { } - if err = otelv1beta1.SetupCollectorWebhook(mgr, cfg, reviewer, crdMetrics); err != nil { + bv := func(collector otelv1beta1.OpenTelemetryCollector) admission.Warnings { + var warnings admission.Warnings + params, newErr := collectorReconciler.GetParams(collector) + if err != nil { + warnings = append(warnings, newErr.Error()) + return warnings + } + _, newErr = collectorManifests.Build(params) + if newErr != nil { + warnings = append(warnings, newErr.Error()) + return warnings + } + return warnings + } + + if err = otelv1beta1.SetupCollectorWebhook(mgr, cfg, reviewer, crdMetrics, bv); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "OpenTelemetryCollector") os.Exit(1) } diff --git a/pkg/collector/upgrade/suite_test.go b/pkg/collector/upgrade/suite_test.go index fdafdca245..89a56d8b40 100644 --- a/pkg/collector/upgrade/suite_test.go +++ b/pkg/collector/upgrade/suite_test.go @@ -105,7 +105,7 @@ func TestMain(m *testing.M) { } reviewer := rbac.NewReviewer(clientset) - if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil); err != nil { + if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil); err != nil { fmt.Printf("failed to SetupWebhookWithManager: %v", err) os.Exit(1) }