Skip to content

Commit 48a8e6f

Browse files
author
Per Goncalves da Silva
committed
add webhook bundle validators
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent 1171691 commit 48a8e6f

File tree

3 files changed

+778
-1
lines changed

3 files changed

+778
-1
lines changed

internal/operator-controller/rukpak/render/validators/validator.go

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
package validators
22

33
import (
4+
"cmp"
45
"errors"
56
"fmt"
7+
"maps"
68
"slices"
9+
"strings"
710

811
"k8s.io/apimachinery/pkg/util/sets"
912

13+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
14+
1015
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
1116
)
1217

@@ -19,6 +24,10 @@ var RegistryV1BundleValidator = render.BundleValidator{
1924
CheckCRDResourceUniqueness,
2025
CheckOwnedCRDExistence,
2126
CheckPackageNameNotEmpty,
27+
CheckWebhookDeploymentReferentialIntegrity,
28+
CheckWebhookNameUniqueness,
29+
CheckConversionWebhookCRDReferenceUniqueness,
30+
CheckConversionWebhooksReferenceOwnedCRDs,
2231
}
2332

2433
// CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name.
@@ -86,3 +95,144 @@ func CheckPackageNameNotEmpty(rv1 *render.RegistryV1) []error {
8695
}
8796
return nil
8897
}
98+
99+
// CheckWebhookSupport checks that if the bundle cluster service version declares webhook definitions
100+
// that it is a singleton operator, i.e. that it only supports AllNamespaces mode. This keeps parity
101+
// with OLMv0 behavior for conversion webhooks,
102+
// https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L193
103+
// Since OLMv1 considers APIs to be cluster-scoped, we initially extend this constraint to validating and mutating webhooks.
104+
// While this might restrict the number of supported bundles, we can tackle the issue of relaxing this constraint in turn
105+
// after getting the webhook support working.
106+
func CheckWebhookSupport(rv1 *render.RegistryV1) []error {
107+
if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
108+
supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{}
109+
for _, mode := range rv1.CSV.Spec.InstallModes {
110+
supportedInstallModes.Insert(mode.Type)
111+
}
112+
if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
113+
return []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}
114+
}
115+
}
116+
117+
return nil
118+
}
119+
120+
// CheckWebhookDeploymentReferentialIntegrity checks that each webhook definition in the csv
121+
// references an existing strategy deployment spec. Errors are sorted by strategy deployment spec name,
122+
// webhook type, and webhook name.
123+
func CheckWebhookDeploymentReferentialIntegrity(rv1 *render.RegistryV1) []error {
124+
webhooksByDeployment := map[string][]v1alpha1.WebhookDescription{}
125+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
126+
webhooksByDeployment[wh.DeploymentName] = append(webhooksByDeployment[wh.DeploymentName], wh)
127+
}
128+
129+
for _, depl := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
130+
delete(webhooksByDeployment, depl.Name)
131+
}
132+
133+
var errs []error
134+
// Loop through sorted keys to keep error messages ordered by deployment name
135+
for _, deploymentName := range slices.Sorted(maps.Keys(webhooksByDeployment)) {
136+
webhookDefns := webhooksByDeployment[deploymentName]
137+
slices.SortFunc(webhookDefns, func(a, b v1alpha1.WebhookDescription) int {
138+
return cmp.Or(cmp.Compare(a.Type, b.Type), cmp.Compare(a.GenerateName, b.GenerateName))
139+
})
140+
for _, webhookDef := range webhookDefns {
141+
errs = append(errs, fmt.Errorf("webhook '%s' of type '%s' references non-existent deployment '%s'", webhookDef.GenerateName, webhookDef.Type, webhookDef.DeploymentName))
142+
}
143+
}
144+
return errs
145+
}
146+
147+
// CheckWebhookNameUniqueness checks that each webhook definition of each type (validating, mutating, or conversion)
148+
// has a unique name. Webhooks of different types can have the same name. Errors are sorted by webhook type
149+
// and name.
150+
func CheckWebhookNameUniqueness(rv1 *render.RegistryV1) []error {
151+
webhookNameSetByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{}
152+
duplicateWebhooksByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{}
153+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
154+
if _, ok := webhookNameSetByType[wh.Type]; !ok {
155+
webhookNameSetByType[wh.Type] = sets.Set[string]{}
156+
}
157+
if webhookNameSetByType[wh.Type].Has(wh.GenerateName) {
158+
if _, ok := duplicateWebhooksByType[wh.Type]; !ok {
159+
duplicateWebhooksByType[wh.Type] = sets.Set[string]{}
160+
}
161+
duplicateWebhooksByType[wh.Type].Insert(wh.GenerateName)
162+
}
163+
webhookNameSetByType[wh.Type].Insert(wh.GenerateName)
164+
}
165+
166+
var errs []error
167+
for _, whType := range slices.Sorted(maps.Keys(duplicateWebhooksByType)) {
168+
for _, webhookName := range slices.Sorted(slices.Values(duplicateWebhooksByType[whType].UnsortedList())) {
169+
errs = append(errs, fmt.Errorf("duplicate webhook '%s' of type '%s'", webhookName, whType))
170+
}
171+
}
172+
return errs
173+
}
174+
175+
// CheckConversionWebhooksReferenceOwnedCRDs checks defined conversion webhooks reference bundle owned CRDs.
176+
// Errors are sorted by webhook name and CRD name.
177+
func CheckConversionWebhooksReferenceOwnedCRDs(rv1 *render.RegistryV1) []error {
178+
//nolint:prealloc
179+
var conversionWebhooks []v1alpha1.WebhookDescription
180+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
181+
if wh.Type != v1alpha1.ConversionWebhook {
182+
continue
183+
}
184+
conversionWebhooks = append(conversionWebhooks, wh)
185+
}
186+
187+
if len(conversionWebhooks) == 0 {
188+
return nil
189+
}
190+
191+
ownedCRDNames := sets.Set[string]{}
192+
for _, crd := range rv1.CSV.Spec.CustomResourceDefinitions.Owned {
193+
ownedCRDNames.Insert(crd.Name)
194+
}
195+
196+
slices.SortFunc(conversionWebhooks, func(a, b v1alpha1.WebhookDescription) int {
197+
return cmp.Compare(a.GenerateName, b.GenerateName)
198+
})
199+
200+
var errs []error
201+
for _, webhook := range conversionWebhooks {
202+
webhookCRDs := webhook.ConversionCRDs
203+
slices.Sort(webhookCRDs)
204+
for _, crd := range webhookCRDs {
205+
if !ownedCRDNames.Has(crd) {
206+
errs = append(errs, fmt.Errorf("conversion webhook '%s' references custom resource definition '%s' not owned bundle", webhook.GenerateName, crd))
207+
}
208+
}
209+
}
210+
return errs
211+
}
212+
213+
// CheckConversionWebhookCRDReferenceUniqueness checks no two (or more) conversion webhooks reference the same CRD.
214+
func CheckConversionWebhookCRDReferenceUniqueness(rv1 *render.RegistryV1) []error {
215+
// collect webhooks by crd
216+
crdToWh := map[string][]string{}
217+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
218+
if wh.Type != v1alpha1.ConversionWebhook {
219+
continue
220+
}
221+
for _, crd := range wh.ConversionCRDs {
222+
crdToWh[crd] = append(crdToWh[crd], wh.GenerateName)
223+
}
224+
}
225+
226+
// remove crds with single webhook
227+
maps.DeleteFunc(crdToWh, func(crd string, whs []string) bool {
228+
return len(whs) == 1
229+
})
230+
231+
errs := make([]error, 0, len(crdToWh))
232+
orderedCRDs := slices.Sorted(maps.Keys(crdToWh))
233+
for _, crd := range orderedCRDs {
234+
orderedWhs := strings.Join(slices.Sorted(slices.Values(crdToWh[crd])), ",")
235+
errs = append(errs, fmt.Errorf("conversion webhooks [%s] reference same custom resource definition '%s'", orderedWhs, crd))
236+
}
237+
return errs
238+
}

0 commit comments

Comments
 (0)