Skip to content

Commit 21c778a

Browse files
arturshadnikTylerGillson
arturshadnik
and
TylerGillson
authored
✨ implement addon create and delete reconcile on the hub (#28)
* refactor: standardize API conventions (#31) * refactor: use pointers only for optional fields, use +optional in conjunction with all omitemptys, tidy code Signed-off-by: Tyler Gillson <[email protected]> * chore: clarify webhook for hosted mode Signed-off-by: Tyler Gillson <[email protected]> * fix: force specification of either clusterManager or singleton Signed-off-by: Tyler Gillson <[email protected]> * chore: make reviewable Signed-off-by: Tyler Gillson <[email protected]> --------- Signed-off-by: Tyler Gillson <[email protected]> * feat: implement addon create and delete reconcile on the hub Signed-off-by: Artur Shad Nik <[email protected]> * chore: address review comments Signed-off-by: Artur Shad Nik <[email protected]> * refactor: do all CMA purging at the end Signed-off-by: Artur Shad Nik <[email protected]> * chore: clarify comment Signed-off-by: Artur Shad Nik <[email protected]> * fix: update manager rbac Signed-off-by: Artur Shad Nik <[email protected]> * feat: use separate CM for each addon's manifests Signed-off-by: Artur Shad Nik <[email protected]> * chore: error wording Signed-off-by: Artur Shad Nik <[email protected]> * chore: try to delete all addons before returning errors Signed-off-by: Artur Shad Nik <[email protected]> * fix: move err return to prevent orphaning addon CRs Signed-off-by: Artur Shad Nik <[email protected]> * chore: parse URLs using lib; use different keys depending on manifest source Signed-off-by: Artur Shad Nik <[email protected]> * chore: make reviewable Signed-off-by: Artur Shad Nik <[email protected]> * chore: make reviewable Signed-off-by: Artur Shad Nik <[email protected]> * chore: add annotations and labels to CMs Signed-off-by: Artur Shad Nik <[email protected]> * feat: add addon manifest validation to fcc webhook Signed-off-by: Artur Shad Nik <[email protected]> * chore: readibility Signed-off-by: Artur Shad Nik <[email protected]> * fix: dont generate deepcopy methods for custom validator Signed-off-by: Artur Shad Nik <[email protected]> * fix: dont use pointer for addonconfig Signed-off-by: Artur Shad Nik <[email protected]> --------- Signed-off-by: Tyler Gillson <[email protected]> Signed-off-by: Artur Shad Nik <[email protected]> Co-authored-by: Tyler Gillson <[email protected]>
1 parent 6a38ee3 commit 21c778a

33 files changed

+5806
-30
lines changed

‎fleetconfig-controller/api/v1alpha1/constants.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,18 @@ const (
6262
// AWSIRSARegistrationDriver is the AWS IAM Role for Service Accounts (IRSA) registration driver.
6363
AWSIRSARegistrationDriver = "awsirsa"
6464
)
65+
66+
// Addon ConfigMap constants
67+
const (
68+
// AddonConfigMapNamePrefix is the common name prefix for all configmaps containing addon configurations.
69+
AddonConfigMapNamePrefix = "fleet-addon"
70+
71+
// AddonConfigMapManifestRawKey is the data key containing raw manifests.
72+
AddonConfigMapManifestRawKey = "manifestsRaw"
73+
74+
// AddonConfigMapManifestRawKey is the data key containing a URL to download manifests.
75+
AddonConfigMapManifestURLKey = "manifestsURL"
76+
)
77+
78+
// AllowedAddonURLSchemes are the URL schemes which can be used to provide manifests for configuring addons.
79+
var AllowedAddonURLSchemes = []string{"http", "https"}

‎fleetconfig-controller/api/v1alpha1/fleetconfig_webhook.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/apimachinery/pkg/util/validation/field"
2626
operatorv1 "open-cluster-management.io/api/operator/v1"
2727
ctrl "sigs.k8s.io/controller-runtime"
28+
"sigs.k8s.io/controller-runtime/pkg/client"
2829
logf "sigs.k8s.io/controller-runtime/pkg/log"
2930
"sigs.k8s.io/controller-runtime/pkg/webhook"
3031
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -37,7 +38,7 @@ var log = logf.Log.WithName("fleetconfig-resource")
3738
func SetupFleetConfigWebhookWithManager(mgr ctrl.Manager) error {
3839
return ctrl.NewWebhookManagedBy(mgr).For(&FleetConfig{}).
3940
WithDefaulter(&FleetConfigCustomDefaulter{}).
40-
WithValidator(&FleetConfigCustomValidator{}).
41+
WithValidator(&FleetConfigCustomValidator{client: mgr.GetClient()}).
4142
Complete()
4243
}
4344

@@ -69,20 +70,21 @@ func (d *FleetConfigCustomDefaulter) Default(_ context.Context, obj runtime.Obje
6970
// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
7071
// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
7172
// +kubebuilder:webhook:path=/validate-fleetconfig-open-cluster-management-io-v1alpha1-fleetconfig,mutating=false,failurePolicy=fail,sideEffects=None,groups=fleetconfig.open-cluster-management.io,resources=fleetconfigs,verbs=create;update;delete,versions=v1alpha1,name=vfleetconfig-v1alpha1.open-cluster-management.io,admissionReviewVersions=v1
73+
// +kubebuilder:object:generate=false
7274

7375
// FleetConfigCustomValidator struct is responsible for validating the FleetConfig resource
7476
// when it is created, updated, or deleted.
7577
//
7678
// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
7779
// as this struct is used only for temporary operations and does not need to be deeply copied.
7880
type FleetConfigCustomValidator struct {
79-
// TODO(user): Add more fields as needed for validation
81+
client client.Client
8082
}
8183

8284
var _ webhook.CustomValidator = &FleetConfigCustomValidator{}
8385

8486
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
85-
func (v *FleetConfigCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
87+
func (v *FleetConfigCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
8688
fc, ok := obj.(*FleetConfig)
8789
if !ok {
8890
return nil, fmt.Errorf("expected a FleetConfig object but got %T", obj)
@@ -128,6 +130,8 @@ func (v *FleetConfigCustomValidator) ValidateCreate(_ context.Context, obj runti
128130
}
129131
}
130132

133+
allErrs = append(allErrs, validateAddonConfigs(ctx, v.client, fc)...)
134+
131135
if len(allErrs) > 0 {
132136
return warnings, errors.NewInvalid(GroupKind, fc.Name, allErrs)
133137
}
@@ -146,7 +150,7 @@ func isKubeconfigValid(kubeconfig Kubeconfig) (bool, string) {
146150
}
147151

148152
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
149-
func (v *FleetConfigCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
153+
func (v *FleetConfigCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
150154
fc, ok := newObj.(*FleetConfig)
151155
if !ok {
152156
return nil, fmt.Errorf("expected a FleetConfig object for the newObj but got %T", newObj)
@@ -162,6 +166,11 @@ func (v *FleetConfigCustomValidator) ValidateUpdate(_ context.Context, oldObj, n
162166
return nil, err
163167
}
164168

169+
errs := validateAddonConfigs(ctx, v.client, fc)
170+
if len(errs) > 0 {
171+
return nil, errors.NewInvalid(GroupKind, fc.Name, errs)
172+
}
173+
165174
log.Info("validation for FleetConfig update allowed", "name", fc.GetName())
166175
return nil, nil
167176
}

‎fleetconfig-controller/api/v1alpha1/validation.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,26 @@
11
package v1alpha1
22

33
import (
4+
"context"
45
"errors"
56
"fmt"
7+
"net/url"
68
"reflect"
9+
"slices"
10+
11+
corev1 "k8s.io/api/core/v1"
12+
"k8s.io/apimachinery/pkg/types"
13+
"k8s.io/apimachinery/pkg/util/validation/field"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
715
)
816

917
// allowFleetConfigUpdate validates the FleetConfig update object to determine if the update action is valid.
1018
// Only the following updates are allowed:
19+
// - spec.addOnConfig
1120
// - spec.registrationAuth.*
1221
// - spec.hub.clusterManager.source.*
1322
// - spec.spokes[*].klusterlet.source.*
23+
// - spec.spokes[*].addOns
1424
func allowFleetConfigUpdate(newObject *FleetConfig, oldObject *FleetConfig) error {
1525

1626
// Hub check
@@ -55,3 +65,40 @@ func allowFleetConfigUpdate(newObject *FleetConfig, oldObject *FleetConfig) erro
5565

5666
return nil
5767
}
68+
69+
func validateAddonConfigs(ctx context.Context, client client.Client, newObject *FleetConfig) field.ErrorList {
70+
errs := field.ErrorList{}
71+
for i, a := range newObject.Spec.AddOnConfigs {
72+
cm := corev1.ConfigMap{}
73+
cmName := fmt.Sprintf("%s-%s-%s", AddonConfigMapNamePrefix, a.Name, a.Version)
74+
err := client.Get(ctx, types.NamespacedName{Name: cmName, Namespace: newObject.Namespace}, &cm)
75+
if err != nil {
76+
errs = append(errs, field.InternalError(field.NewPath("addOnConfigs").Index(i), err))
77+
}
78+
// Extract manifest configuration from ConfigMap
79+
_, hasRaw := cm.Data[AddonConfigMapManifestRawKey]
80+
manifestsURL, hasURL := cm.Data[AddonConfigMapManifestURLKey]
81+
82+
// Validate manifest configuration
83+
if !hasRaw && !hasURL {
84+
// return fmt.Errorf("no inline manifests or URL found for addon %s version %s", a.Name, a.Version)
85+
errs = append(errs, field.Invalid(field.NewPath("addOnConfigs").Index(i), a.Name, fmt.Sprintf("no inline manifests or URL found for addon %s version %s", a.Name, a.Version)))
86+
}
87+
if hasRaw && hasURL {
88+
errs = append(errs, field.Invalid(field.NewPath("addOnConfigs").Index(i), a.Name, fmt.Sprintf("only 1 of inline manifests or URL can be set for addon %s version %s", a.Name, a.Version)))
89+
}
90+
91+
if hasURL {
92+
url, err := url.Parse(manifestsURL)
93+
if err != nil {
94+
errs = append(errs, field.Invalid(field.NewPath("addOnConfigs").Index(i), a.Name, fmt.Sprintf("invalid URL '%s' for addon %s version %s. %v", manifestsURL, a.Name, a.Version, err.Error())))
95+
continue
96+
}
97+
if !slices.Contains(AllowedAddonURLSchemes, url.Scheme) {
98+
errs = append(errs, field.Invalid(field.NewPath("addOnConfigs").Index(i), a.Name, fmt.Sprintf("unsupported URL scheme %s for addon %s version %s. Must be one of %v", manifestsURL, a.Name, a.Version, AllowedAddonURLSchemes)))
99+
}
100+
}
101+
}
102+
103+
return errs
104+
}

‎fleetconfig-controller/api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 0 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎fleetconfig-controller/charts/fleetconfig-controller/templates/ocm/add-ons.yaml‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
1-
{{- if .Values.fleetConfig.addOns }}
2-
{{ $addOns := .Values.fleetConfig.addOns }}
1+
{{- if .Values.fleetConfig.addOnConfigs }}
2+
{{- $namespace := .Release.Namespace }}
3+
{{- range .Values.fleetConfig.addOnConfigs }}
4+
{{ $versionedName := printf "%s-%s" .name .version }}
35
apiVersion: v1
46
kind: ConfigMap
57
metadata:
6-
name: fleet-add-ons
7-
namespace: {{ .Release.Namespace }}
8+
name: fleet-addon-{{ $versionedName }}
9+
namespace: {{ $namespace }}
10+
annotations:
11+
{{- include "chart.annotations" . | nindent 4 }}
12+
helm.sh/hook: pre-install,pre-upgrade
13+
labels:
14+
{{- include "chart.labels" . | nindent 4 }}
815
data:
9-
{{- range $addOns }}
10-
{{- if or (hasPrefix "http://" .manifests) (hasPrefix "https://" .manifests) (hasPrefix "oci://" .manifests) }}
11-
{{ .name }}: {{ .manifests }}
16+
{{- if or (hasPrefix "http://" .manifests) (hasPrefix "https://" .manifests) }}
17+
manifestsURL: {{ .manifests }}
1218
{{- else }}
13-
{{ .name }}: |-
19+
manifestsRaw: |-
1420
{{- .manifests | nindent 4 }}
1521
{{- end }}
16-
{{- end }}
22+
---
23+
{{- end }}
1724
{{- end }}

‎fleetconfig-controller/charts/fleetconfig-controller/templates/rbac/manager-rbac.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ rules:
8888
resources:
8989
- "addondeploymentconfigs"
9090
- "addontemplates"
91-
verbs: ["get", "list", "watch"]
91+
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
9292
- apiGroups: ["addon.open-cluster-management.io"]
9393
resources:
9494
- "clustermanagementaddons"

‎fleetconfig-controller/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ require (
99
github.com/onsi/gomega v1.37.0
1010
github.com/openshift/build-machinery-go v0.0.0-20250602125535-1b6d00b8c37c
1111
github.com/openshift/imagebuilder v1.2.16
12+
github.com/pkg/errors v0.9.1
1213
k8s.io/api v0.33.1
1314
k8s.io/apimachinery v0.33.1
1415
k8s.io/client-go v0.33.1
@@ -76,7 +77,6 @@ require (
7677
github.com/opencontainers/go-digest v1.0.0 // indirect
7778
github.com/opencontainers/image-spec v1.1.0 // indirect
7879
github.com/opencontainers/runtime-spec v1.2.0 // indirect
79-
github.com/pkg/errors v0.9.1 // indirect
8080
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
8181
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
8282
github.com/prometheus/client_golang v1.22.0 // indirect

0 commit comments

Comments
 (0)