diff --git a/internal/module/module.go b/internal/module/module.go index c3ce7c31..158ee4d5 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -340,7 +340,8 @@ func mapTemplatesRules(linterSettings *pkg.LintersSettings, configSettings *conf rules.ServicePortRule.SetLevel(globalRules.ServicePortRule.Impact, fallbackImpact) rules.ClusterDomainRule.SetLevel(globalRules.ClusterDomainRule.Impact, fallbackImpact) rules.RegistryRule.SetLevel(globalRules.RegistryRule.Impact, fallbackImpact) - rules.EnabledModulesRule.SetLevel(globalRules.EnabledModulesRule.Impact, fallbackImpact) + + rules.WebhookConfigurationRule.SetLevel(globalRules.WebhookConfigurationRule.Impact, fallbackImpact) } // mapOpenAPIRules configures OpenAPI linter rules @@ -455,6 +456,7 @@ func mapTemplatesExclusionsAndSettings(linterSettings *pkg.LintersSettings, conf excludes.Ingress = configExcludes.Ingress.Get() excludes.EnabledModules.Files = pkg.StringRuleExcludeList(configExcludes.EnabledModules.Files) excludes.EnabledModules.Directories = pkg.DirectoryRuleExcludeList(configExcludes.EnabledModules.Directories) + excludes.WebhookConfiguration = configExcludes.WebhookConfiguration.Get() // Additional settings linterSettings.Templates.PrometheusRuleSettings.Disable = configSettings.Templates.PrometheusRules.Disable diff --git a/pkg/config.go b/pkg/config.go index d5e9872a..0cd19c3c 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -131,17 +131,18 @@ type TemplatesLinterConfig struct { GrafanaDashboardsSettings GrafanaDashboardsSettings } type TemplatesLinterRules struct { - VPARule RuleConfig - PDBRule RuleConfig - IngressRule RuleConfig - PrometheusRule RuleConfig - GrafanaRule RuleConfig - KubeRBACProxyRule RuleConfig - ServicePortRule RuleConfig - ClusterDomainRule RuleConfig - RegistryRule RuleConfig - HTTPRouteRule RuleConfig - EnabledModulesRule RuleConfig + VPARule RuleConfig + PDBRule RuleConfig + IngressRule RuleConfig + PrometheusRule RuleConfig + GrafanaRule RuleConfig + KubeRBACProxyRule RuleConfig + ServicePortRule RuleConfig + ClusterDomainRule RuleConfig + RegistryRule RuleConfig + HTTPRouteRule RuleConfig + EnabledModulesRule RuleConfig + WebhookConfigurationRule RuleConfig } type PrometheusRuleSettings struct { @@ -152,13 +153,14 @@ type GrafanaDashboardsSettings struct { Disable bool } type TemplatesExcludeRules struct { - VPAAbsent KindRuleExcludeList - PDBAbsent KindRuleExcludeList - ServicePort ServicePortExcludeList - KubeRBACProxy StringRuleExcludeList - Ingress KindRuleExcludeList - HTTPRoute KindRuleExcludeList - EnabledModules EnabledModulesExcludeRule + VPAAbsent KindRuleExcludeList + PDBAbsent KindRuleExcludeList + ServicePort ServicePortExcludeList + KubeRBACProxy StringRuleExcludeList + Ingress KindRuleExcludeList + HTTPRoute KindRuleExcludeList + EnabledModules EnabledModulesExcludeRule + WebhookConfiguration KindRuleExcludeList } type EnabledModulesExcludeRule struct { diff --git a/pkg/config/global/global.go b/pkg/config/global/global.go index d6c969a9..d7a86597 100644 --- a/pkg/config/global/global.go +++ b/pkg/config/global/global.go @@ -129,16 +129,17 @@ type TemplatesLinterConfig struct { } type TemplatesLinterRules struct { - VPARule RuleConfig `mapstructure:"vpa"` - PDBRule RuleConfig `mapstructure:"pdb"` - IngressRule RuleConfig `mapstructure:"ingress"` - PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` - GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` - KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` - ServicePortRule RuleConfig `mapstructure:"service-port"` - ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` - RegistryRule RuleConfig `mapstructure:"registry"` - EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + VPARule RuleConfig `mapstructure:"vpa"` + PDBRule RuleConfig `mapstructure:"pdb"` + IngressRule RuleConfig `mapstructure:"ingress"` + PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` + GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` + KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` + ServicePortRule RuleConfig `mapstructure:"service-port"` + ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` + RegistryRule RuleConfig `mapstructure:"registry"` + EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` } func (c LinterConfig) IsWarn() bool { diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 0f4b7c75..5753f3ab 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -218,25 +218,27 @@ type TemplatesSettings struct { } type TemplatesLinterRules struct { - VPARule RuleConfig `mapstructure:"vpa"` - PDBRule RuleConfig `mapstructure:"pdb"` - IngressRule RuleConfig `mapstructure:"ingress"` - PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` - GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` - KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` - ServicePortRule RuleConfig `mapstructure:"service-port"` - ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` - RegistryRule RuleConfig `mapstructure:"registry"` - EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + VPARule RuleConfig `mapstructure:"vpa"` + PDBRule RuleConfig `mapstructure:"pdb"` + IngressRule RuleConfig `mapstructure:"ingress"` + PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` + GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` + KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` + ServicePortRule RuleConfig `mapstructure:"service-port"` + ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` + RegistryRule RuleConfig `mapstructure:"registry"` + EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` } type TemplatesExcludeRules struct { - VPAAbsent KindRuleExcludeList `mapstructure:"vpa"` - PDBAbsent KindRuleExcludeList `mapstructure:"pdb"` - ServicePort ServicePortExcludeList `mapstructure:"service-port"` - KubeRBACProxy StringRuleExcludeList `mapstructure:"kube-rbac-proxy"` - Ingress KindRuleExcludeList `mapstructure:"ingress"` - EnabledModules EnabledModulesExcludeRule `mapstructure:"enabled-modules"` + VPAAbsent KindRuleExcludeList `mapstructure:"vpa"` + PDBAbsent KindRuleExcludeList `mapstructure:"pdb"` + ServicePort ServicePortExcludeList `mapstructure:"service-port"` + KubeRBACProxy StringRuleExcludeList `mapstructure:"kube-rbac-proxy"` + Ingress KindRuleExcludeList `mapstructure:"ingress"` + EnabledModules EnabledModulesExcludeRule `mapstructure:"enabled-modules"` + WebhookConfiguration KindRuleExcludeList `mapstructure:"webhook-configuration-annotations"` } type EnabledModulesExcludeRule struct { diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index 62b0aabe..b5a8291a 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -22,6 +22,7 @@ Proper template validation prevents runtime issues, ensures applications are pro | [registry](#registry) | Validates registry secret configuration | ❌ | enabled | | [werf](#werf) | Validates image names in `werf.yaml` do not contain underscores | ❌ | enabled | | [enabled-modules](#enabled-modules) | Detects usage of `.Values.global.enabledModules` in templates | ✅ | enabled | +| [webhook-configuration-annotations](#webhook-configuration-annotations) | Checks webhook configurations have werf.io/weight or deploy-dependency annotations | ✅ | enabled | ## Rule Details @@ -1850,6 +1851,110 @@ linters-settings: - templates/vendor/ # Exclude entire directory ``` +### webhook-configuration-annotations + +**Purpose:** Ensures every `ValidatingWebhookConfiguration` and `MutatingWebhookConfiguration` has at least one ordering annotation: `werf.io/weight` or an annotation with the `werf.io/deploy-dependency-` prefix (e.g. `werf.io/deploy-dependency-deployment`, `werf.io/deploy-dependency-service`). These annotations control werf deploy ordering: `werf.io/deploy-dependency-*` declares a dependency on another resource (the recommended approach), while `werf.io/weight` sets explicit ordering priority. + +**Description:** + +Iterates all parsed Kubernetes resources, filters for webhook configuration kinds, and checks that each webhook configuration declares its position in the deploy order via annotations. Without these annotations, webhook configurations may deploy in an undefined order, potentially causing cluster API disruptions. + +**What it checks:** + +1. Every `ValidatingWebhookConfiguration` has either `werf.io/weight` or an annotation starting with `werf.io/deploy-dependency-` +2. Every `MutatingWebhookConfiguration` has either `werf.io/weight` or an annotation starting with `werf.io/deploy-dependency-` +3. Note: `werf.io/deploy-on` alone is not sufficient — it controls deploy *stages*, not deploy *ordering* +4. Resources with neither annotation are reported as errors (configurable via `impact`, set to `warn` to downgrade) + +**Why it matters:** + +Webhook backing services (its Deployment, Service, etc) should be deployed before the webhook itself (MutatingWebhookConfiguration or ValidationWebhookConfiguration). Otherwise, if the module rollout was not finished properly (network issues, OOM and so on), the cluster might be left in a state where webhook is deployed, but has no backing services. And if so, resources that this webhook validates/mutates could not be created or updated anymore. To avoid this, deployment order of the webhook and its backing services must be enforced via annotations. + +**Examples:** + +❌ **Incorrect** - Webhook configuration without ordering annotations: + +```yaml +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: my-webhook +webhooks: + - name: check.example.com + clientConfig: + service: + name: my-service + namespace: d8-my-module + rules: + - operations: ["CREATE", "UPDATE"] + apiGroups: [""] + apiVersions: ["v1"] + resources: ["pods"] +``` + +**Error:** +``` +ValidatingWebhookConfiguration "my-webhook" must have either "werf.io/deploy-dependency" or "werf.io/weight" annotation +``` + +✅ **Correct** - With deploy ordering annotations: + +```yaml +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: my-webhook + annotations: + werf.io/deploy-dependency-deployment: state=ready,kind=Deployment,name=my-app,namespace=d8-my-module + werf.io/deploy-dependency-service: state=present,kind=Service,name=my-svc,namespace=d8-my-module +webhooks: + - name: check.example.com + ... +``` + +✅ **Correct** - With weight annotation only: + +```yaml +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: my-webhook + annotations: + werf.io/weight: "10" +webhooks: + - name: check.example.com + ... +``` + +**Configuration:** + +The rule defaults to `warning` level and can be configured via global or module `.dmtlint.yaml`. + +**Impact level** — set the severity of the check: + +```yaml +# .dmtlint.yaml (global) or /.dmtlint.yaml +linters-settings: + templates: + rules: + webhook-configuration-annotations: + impact: warn # error | warn (default: error) +``` + +**Excluding resources** — skip specific webhook configurations by kind and name: + +```yaml +# .dmtlint.yaml (global) or /.dmtlint.yaml +linters-settings: + templates: + exclude-rules: + webhook-configuration-annotations: + - kind: ValidatingWebhookConfiguration + name: istio-sidecar-injector # managed externally by istio operator + - kind: MutatingWebhookConfiguration + name: cert-manager-webhook # managed externally by cert-manager operator +``` + ## Configuration The Templates linter can be configured at the module level with rule-specific settings and exclusions. @@ -1904,6 +2009,8 @@ linters-settings: impact: error enabled-modules: impact: warning + webhook-configuration-annotations: + impact: error ``` ### Rule-Level Exclusions @@ -1961,6 +2068,13 @@ linters-settings: - templates/legacy-deployment.yaml directories: - templates/vendor/ + + # webhook-configuration-annotations exclusions (by kind and name) + webhook-configuration-annotations: + - kind: ValidatingWebhookConfiguration + name: istio-sidecar-injector + - kind: MutatingWebhookConfiguration + name: cert-manager-webhook ``` ### Complete Configuration Example @@ -1979,6 +2093,11 @@ linters-settings: prometheus-rules: disable: false + # Rule-specific impact levels + rules: + webhook-configuration-annotations: + impact: warn # downgrade from default error to warn + # Rule-specific exclusions exclude-rules: vpa: @@ -2003,6 +2122,10 @@ linters-settings: kube-rbac-proxy: - d8-development + + webhook-configuration-annotations: + - kind: ValidatingWebhookConfiguration + name: istio-sidecar-injector ``` ### Configuration in Module Directory diff --git a/pkg/linters/templates/rules/webhook_configuration.go b/pkg/linters/templates/rules/webhook_configuration.go new file mode 100644 index 00000000..91f664b9 --- /dev/null +++ b/pkg/linters/templates/rules/webhook_configuration.go @@ -0,0 +1,88 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "strings" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + WebhookConfigurationRuleName = "webhook-configuration-annotations" + + AnnotationWeight = "werf.io/weight" + AnnotationDeployDependency = "werf.io/deploy-dependency" + AnnotationDeployDependencyPrefix = "werf.io/deploy-dependency-" +) + +func NewWebhookConfigurationRule(excludeRules []pkg.KindRuleExclude) *WebhookConfigurationRule { + return &WebhookConfigurationRule{ + RuleMeta: pkg.RuleMeta{ + Name: WebhookConfigurationRuleName, + }, + KindRule: pkg.KindRule{ + ExcludeRules: excludeRules, + }, + } +} + +type WebhookConfigurationRule struct { + pkg.RuleMeta + pkg.KindRule +} + +// hasDeployDependencyAnnotation checks whether any annotation key starts with +// "werf.io/deploy-dependency". In practice werf uses suffixed keys such as +// "werf.io/deploy-dependency-deployment" or "werf.io/deploy-dependency-service", +// so a prefix match is required instead of an exact key match. +func hasDeployDependencyAnnotation(annotations map[string]string) bool { + for key := range annotations { + if strings.HasPrefix(key, AnnotationDeployDependencyPrefix) { + return true + } + } + + return false +} + +func (r *WebhookConfigurationRule) ValidateWebhookConfigurationAnnotations(m pkg.Module, errorList *errors.LintRuleErrorsList) { + errorList = errorList.WithRule(r.GetName()) + + for _, object := range m.GetStorage() { + kind := object.Unstructured.GetKind() + if kind != "ValidatingWebhookConfiguration" && kind != "MutatingWebhookConfiguration" { + continue + } + + if !r.Enabled(kind, object.Unstructured.GetName()) { + continue + } + + annotations := object.Unstructured.GetAnnotations() + + _, hasWeight := annotations[AnnotationWeight] + hasDeployDependency := hasDeployDependencyAnnotation(annotations) + + if !hasWeight && !hasDeployDependency { + errorList.WithObjectID(object.Identity()). + WithFilePath(object.GetPath()). + Errorf("%s %q must have either %q annotation or an annotation with %q prefix", kind, object.Unstructured.GetName(), AnnotationWeight, AnnotationDeployDependency) + } + } +} diff --git a/pkg/linters/templates/rules/webhook_configuration_test.go b/pkg/linters/templates/rules/webhook_configuration_test.go new file mode 100644 index 00000000..767c98fb --- /dev/null +++ b/pkg/linters/templates/rules/webhook_configuration_test.go @@ -0,0 +1,217 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "testing" + + "github.com/gojuno/minimock/v3" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/deckhouse/dmt/internal/mocks" + "github.com/deckhouse/dmt/internal/storage" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +func makeStorage(kind, name string, annotations map[string]string) map[storage.ResourceIndex]storage.StoreObject { + u := unstructured.Unstructured{} + u.SetKind(kind) + u.SetName(name) + u.SetAnnotations(annotations) + + return map[storage.ResourceIndex]storage.StoreObject{ + {Kind: kind, Name: name}: { + Unstructured: u, + AbsPath: "/test/" + name + ".yaml", + }, + } +} + +func TestWebhookConfigurationRule_OnlyWeight(t *testing.T) { + mc := minimock.NewController(t) + + storage := makeStorage("ValidatingWebhookConfiguration", "my-webhook", map[string]string{ + "werf.io/weight": "10", + }) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.False(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_OnlyDeployDependencySuffixed(t *testing.T) { + mc := minimock.NewController(t) + + storage := makeStorage("ValidatingWebhookConfiguration", "my-webhook", map[string]string{ + "werf.io/deploy-dependency-deployment": "state=ready,kind=Deployment,name=my-app,namespace=d8-module", + "werf.io/deploy-dependency-service": "state=present,kind=Service,name=my-svc,namespace=d8-module", + }) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.False(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_BothWeightAndDeployDependencySuffixed(t *testing.T) { + mc := minimock.NewController(t) + + storage := makeStorage("ValidatingWebhookConfiguration", "my-webhook", map[string]string{ + "werf.io/weight": "10", + "werf.io/deploy-dependency-deployment": "state=ready,kind=Deployment,name=my-app,namespace=d8-module", + }) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.False(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_NeitherAnnotation(t *testing.T) { + mc := minimock.NewController(t) + + storage := makeStorage("ValidatingWebhookConfiguration", "my-webhook", map[string]string{ + "other-annotation": "value", + }) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.True(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_MutatingWebhookConfiguration(t *testing.T) { + mc := minimock.NewController(t) + + storage := makeStorage("MutatingWebhookConfiguration", "my-hook", map[string]string{ + "werf.io/weight": "5", + }) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.False(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_SkipsNonWebhookResources(t *testing.T) { + mc := minimock.NewController(t) + + u := unstructured.Unstructured{} + u.SetKind("Deployment") + u.SetName("my-deploy") + + storage := map[storage.ResourceIndex]storage.StoreObject{ + {Kind: "Deployment", Name: "my-deploy"}: { + Unstructured: u, + AbsPath: "/test/deploy.yaml", + }, + } + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.False(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_DeployOnNotEnough(t *testing.T) { + mc := minimock.NewController(t) + + // "werf.io/deploy-on" controls deploy *stages* (e.g. pre-install), not + // deploy *ordering* — it should NOT satisfy the rule on its own. + storage := makeStorage("ValidatingWebhookConfiguration", "my-webhook", map[string]string{ + "werf.io/deploy-on": "pre-install,pre-upgrade,pre-rollback", + }) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + rule := NewWebhookConfigurationRule(nil) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.True(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_ExcludedResource(t *testing.T) { + mc := minimock.NewController(t) + + storage := makeStorage("ValidatingWebhookConfiguration", "excluded-hook", map[string]string{}) + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(storage) + + exclude := []pkg.KindRuleExclude{ + {Kind: "ValidatingWebhookConfiguration", Name: "excluded-hook"}, + } + rule := NewWebhookConfigurationRule(exclude) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.False(t, errorList.ContainsErrors()) +} + +func TestWebhookConfigurationRule_ExcludedResourceDoesNotAffectOthers(t *testing.T) { + mc := minimock.NewController(t) + + store := makeStorage("ValidatingWebhookConfiguration", "excluded-hook", nil) + store[storage.ResourceIndex{Kind: "MutatingWebhookConfiguration", Name: "other-hook"}] = func() storage.StoreObject { + u := unstructured.Unstructured{} + u.SetKind("MutatingWebhookConfiguration") + u.SetName("other-hook") + + return storage.StoreObject{Unstructured: u, AbsPath: "/test/other.yaml"} + }() + + mod := mocks.NewModuleMock(mc) + mod.GetStorageMock.Return(store) + + exclude := []pkg.KindRuleExclude{ + {Kind: "ValidatingWebhookConfiguration", Name: "excluded-hook"}, + } + rule := NewWebhookConfigurationRule(exclude) + errorList := errors.NewLintRuleErrorsList() + + rule.ValidateWebhookConfigurationAnnotations(mod, errorList) + assert.True(t, errorList.ContainsErrors()) +} diff --git a/pkg/linters/templates/templates.go b/pkg/linters/templates/templates.go index d4d4363b..1ed22fd7 100644 --- a/pkg/linters/templates/templates.go +++ b/pkg/linters/templates/templates.go @@ -96,6 +96,10 @@ func (l *Templates) Run(m *module.Module) { rules.NewRegistryRule().CheckRegistrySecret(m, errorList.WithMaxLevel(l.cfg.Rules.RegistryRule.GetLevel())) + // WebhookConfiguration annotations rule + rules.NewWebhookConfigurationRule(l.cfg.ExcludeRules.WebhookConfiguration.Get()). + ValidateWebhookConfigurationAnnotations(m, errorList.WithMaxLevel(l.cfg.Rules.WebhookConfigurationRule.GetLevel())) + // EnabledModules rule rules.NewEnabledModulesRule( l.cfg.ExcludeRules.EnabledModules.Files.Get(), diff --git a/test/e2e/testdata/templates/webhook-config-annotations-absent/expected.yaml b/test/e2e/testdata/templates/webhook-config-annotations-absent/expected.yaml new file mode 100644 index 00000000..0680f4c5 --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-absent/expected.yaml @@ -0,0 +1,9 @@ +description: > + A ValidatingWebhookConfiguration without werf.io/weight or werf.io/deploy-dependency-* + annotation must be flagged by the webhook-configuration-annotations rule. +module: module +expect: + - linter: templates + rule: webhook-configuration-annotations + level: error + textContains: "must have either \"werf.io/weight\" annotation or an annotation with \"werf.io/deploy-dependency\" prefix" diff --git a/test/e2e/testdata/templates/webhook-config-annotations-absent/module/module.yaml b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/module.yaml new file mode 100644 index 00000000..953f0c2f --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-webhook-absent +namespace: e2e-webhook-absent diff --git a/test/e2e/testdata/templates/webhook-config-annotations-absent/module/openapi/config-values.yaml b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/webhook-config-annotations-absent/module/openapi/values.yaml b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/webhook-config-annotations-absent/module/templates/validatingwebhook.yaml b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/templates/validatingwebhook.yaml new file mode 100644 index 00000000..882095e6 --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-absent/module/templates/validatingwebhook.yaml @@ -0,0 +1,18 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: e2e-webhook-absent +webhooks: + - name: check.example.com + clientConfig: + service: + name: my-service + namespace: e2e-webhook-absent + caBundle: Cg== + admissionReviewVersions: ["v1"] + sideEffects: None + rules: + - operations: ["CREATE", "UPDATE"] + apiGroups: [""] + apiVersions: ["v1"] + resources: ["pods"] diff --git a/test/e2e/testdata/templates/webhook-config-annotations-present/expected.yaml b/test/e2e/testdata/templates/webhook-config-annotations-present/expected.yaml new file mode 100644 index 00000000..f4f51548 --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-present/expected.yaml @@ -0,0 +1,31 @@ +description: > + A ValidatingWebhookConfiguration with werf.io/weight annotation must pass + the webhook-configuration-annotations rule without errors. Uses exhaustive + matching to confirm the rule produces no findings. +module: module +exhaustive: true +expect: + - linter: container + rule: object-recommended-labels + level: error + textContains: '"module"' + - linter: container + rule: object-recommended-labels + level: error + textContains: '"heritage"' + - linter: module + rule: definition-file + level: error + textContains: "'stage' is required" + - linter: module + rule: definition-file + level: error + textContains: '`descriptions.en` field is required' + - linter: module + rule: helmignore + level: error + textContains: ".helmignore is required" + - linter: documentation + rule: readme + level: error + textContains: "README.md file is missing" diff --git a/test/e2e/testdata/templates/webhook-config-annotations-present/module/module.yaml b/test/e2e/testdata/templates/webhook-config-annotations-present/module/module.yaml new file mode 100644 index 00000000..61d3dd27 --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-present/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-webhook-present +namespace: e2e-webhook-present diff --git a/test/e2e/testdata/templates/webhook-config-annotations-present/module/openapi/config-values.yaml b/test/e2e/testdata/templates/webhook-config-annotations-present/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-present/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/webhook-config-annotations-present/module/openapi/values.yaml b/test/e2e/testdata/templates/webhook-config-annotations-present/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-present/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/webhook-config-annotations-present/module/templates/validatingwebhook.yaml b/test/e2e/testdata/templates/webhook-config-annotations-present/module/templates/validatingwebhook.yaml new file mode 100644 index 00000000..a4a8d07e --- /dev/null +++ b/test/e2e/testdata/templates/webhook-config-annotations-present/module/templates/validatingwebhook.yaml @@ -0,0 +1,20 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: e2e-webhook-present + annotations: + werf.io/weight: "10" +webhooks: + - name: check.example.com + clientConfig: + service: + name: my-service + namespace: e2e-webhook-present + caBundle: Cg== + admissionReviewVersions: ["v1"] + sideEffects: None + rules: + - operations: ["CREATE", "UPDATE"] + apiGroups: [""] + apiVersions: ["v1"] + resources: ["pods"] diff --git a/webhook-rule-ci-failures.md b/webhook-rule-ci-failures.md new file mode 100644 index 00000000..2b4ece6f --- /dev/null +++ b/webhook-rule-ci-failures.md @@ -0,0 +1,66 @@ +# CI Failure Analysis: `webhook-configuration-annotations` rule + +PR: [#412](https://github.com/deckhouse/dmt/pull/412) | Run: [#134](https://github.com/deckhouse/dmt/actions/runs/28246712494) + +## Summary + +The new `webhook-configuration-annotations` rule defaults to **`error`** level. When run against the Deckhouse `main` codebase, it flags **19 webhook resource files across 9 modules** that lack the required `werf.io/weight` or `werf.io/deploy-dependency` annotation. This causes the `DMT Lint Verify` CI job to fail. + +## Affected Modules + +| # | Module | Files | Kind | CODEOWNERS | +|---|--------|-------|------|------------| +| 1 | **002-deckhouse** | `templates/admission/validation.yaml` | ValidatingWebhookConfiguration | @ldmonster | +| 2 | **015-admission-policy-engine** | `templates/validatingwebhookconfiguration.yaml` | ValidatingWebhookConfiguration | @nabokihms @AlwxSin | +| 3 | **015-admission-policy-engine** | `templates/mutatingwebhookconfiguration.yaml` | MutatingWebhookConfiguration | @nabokihms @AlwxSin | +| 4 | **030-cloud-provider-vcd** | `templates/capcd-controller-manager/admission.yaml` | ValidatingWebhookConfiguration, MutatingWebhookConfiguration | @aleksey-su @pabateman | +| 5 | **040-node-manager** | `templates/node-controller/webhook.yaml` | ValidatingWebhookConfiguration | @borg-z @090809 | +| 6 | **040-node-manager** | `templates/caps-controller-manager/webhook.yaml` | ValidatingWebhookConfiguration | @borg-z @090809 | +| 7 | **042-kube-dns** | `templates/sts-pods-hosts-appender-webhook/webhook-configuration.yaml` | MutatingWebhookConfiguration | @AndreyPavlovFlant @apolovov | +| 8 | **101-cert-manager** | `templates/webhook/validatingwebhookconfiguration.yaml` | ValidatingWebhookConfiguration | @Suselz @AlwxSin | +| 9 | **101-cert-manager** | `templates/webhook/mutatingwebhookconfiguration.yaml` | MutatingWebhookConfiguration | @Suselz @AlwxSin | +| 10 | **110-istio** | `templates/control-plane/mutatingwebhook-global.yaml` | MutatingWebhookConfiguration | @skurbatov @apolovov | +| 11 | **110-istio** | `templates/control-plane/validatingwebhook-global.yaml` | ValidatingWebhookConfiguration | @skurbatov @apolovov | +| 12 | **110-istio** | `templates/control-plane/mutatingwebhook-revisions.yaml` | MutatingWebhookConfiguration | @skurbatov @apolovov | +| 13 | **110-istio** | `templates/control-plane/validatingwebhook-revisions.yaml` | ValidatingWebhookConfiguration | @skurbatov @apolovov | +| 14 | **110-istio** | `templates/control-plane/iop/iop.yaml` | ValidatingWebhookConfiguration, MutatingWebhookConfiguration | @skurbatov @apolovov | +| 15 | **110-istio** | `templates/control-plane/iop/istios.yaml` | ValidatingWebhookConfiguration, MutatingWebhookConfiguration | @skurbatov @apolovov | +| 16 | **160-multitenancy-manager** | `templates/admission/validation.yaml` | ValidatingWebhookConfiguration | @nabokihms @AlwxSin | +| 17 | **160-multitenancy-manager** | `templates/cluster-objects-controller/protect-webhook.yaml` | ValidatingWebhookConfiguration | @nabokihms @AlwxSin | +| 18 | **302-vertical-pod-autoscaler** | `templates/admission-controller/mutatingwebhookconfiguration.yaml` | MutatingWebhookConfiguration | @yalosev @ergoz | + +## Already Compliant + +Only one file already has the required annotation: + +| Module | File | Annotation | +|--------|------|------------| +| **040-node-manager** | `templates/capi-controller-manager/webhook.yaml` | `werf.io/deploy-dependency` (×4) | + +## Root Cause + +In `pkg/config.go`, the `RuleConfig.SetLevel()` method defaults to `Error` when no configuration is provided: + +```go +func (rc *RuleConfig) SetLevel(level, backoff string) { + if level != "" { ...; return } + if backoff != "" { ...; return } + lvl := Error // ← hardcoded default + rc.impact = &lvl +} +``` + +Neither the global `.dmtlint.yaml` nor any module-level config specify an impact for `webhook-configuration-annotations`, so the rule always runs at `error` level. + +## Fix Options + +1. **Lower default to `warn`** in `pkg/config.go` — change `lvl := Error` → `lvl := Warn` +2. **Add to Deckhouse global `.dmtlint.yaml`**: + ```yaml + global: + linters-settings: + templates: + rules: + webhook-configuration-annotations: + impact: warn + ```