Skip to content

Commit

Permalink
Update IgnitionSecretRef to ObjectRef (#74)
Browse files Browse the repository at this point in the history
* Update IgnitionSecretRef to ObjectRef

* Ensure Ignition with Ref's Namespace

* Fix the Enqueue method for IgnitionSecrets

* Add Eventfilter for ResourceVersion changes
  • Loading branch information
hardikdr authored Jun 25, 2024
1 parent 1a287eb commit d9873d3
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 22 deletions.
8 changes: 4 additions & 4 deletions api/v1alpha1/httpbootconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (

// HTTPBootConfigSpec defines the desired state of HTTPBootConfig
type HTTPBootConfigSpec struct {
SystemUUID string `json:"systemUUID,omitempty"`
IgnitionSecretRef *corev1.LocalObjectReference `json:"ignitionSecretRef,omitempty"`
SystemIPs []string `json:"systemIPs,omitempty"`
UKIURL string `json:"ukiURL,omitempty"`
SystemUUID string `json:"systemUUID,omitempty"`
IgnitionSecretRef *corev1.ObjectReference `json:"ignitionSecretRef,omitempty"`
SystemIPs []string `json:"systemIPs,omitempty"`
UKIURL string `json:"ukiURL,omitempty"`
}

// HTTPBootConfigStatus defines the observed state of HTTPBootConfig
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

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

59 changes: 51 additions & 8 deletions config/crd/bases/boot.ironcore.dev_httpbootconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,62 @@ spec:
properties:
ignitionSecretRef:
description: |-
LocalObjectReference contains enough information to let you locate the
referenced object inside the same namespace.
ObjectReference contains enough information to let you inspect or modify the referred object.
---
New uses of this type are discouraged because of difficulty describing its usage when embedded in APIs.
1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion and FieldPath are both very rarely valid in actual usage.
2. Invalid usage help. It is impossible to add specific help for individual usage. In most embedded usages, there are particular
restrictions like, "must refer only to types A and B" or "UID not honored" or "name must be restricted".
Those cannot be well described when embedded.
3. Inconsistent validation. Because the usages are different, the validation rules are different by usage, which makes it hard for users to predict what will happen.
4. The fields are both imprecise and overly precise. Kind is not a precise mapping to a URL. This can produce ambiguity
during interpretation and require a REST mapping. In most cases, the dependency is on the group,resource tuple
and the version of the actual struct is irrelevant.
5. We cannot easily change it. Because this type is embedded in many locations, updates to this type
will affect numerous schemas. Don't make new APIs embed an underspecified API type they do not control.
Instead of using this type, create a locally provided and used type that is well-focused on your reference.
For example, ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 .
properties:
apiVersion:
description: API version of the referent.
type: string
fieldPath:
description: |-
If referring to a piece of an object instead of an entire object, this string
should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
For example, if the object reference is to a container within a pod, this would take on a value like:
"spec.containers{name}" (where "name" refers to the name of the container that triggered
the event) or if no container name is specified "spec.containers[2]" (container with
index 2 in this pod). This syntax is chosen only to have some well-defined way of
referencing a part of an object.
TODO: this design is not final and this field is subject to change in the future.
type: string
kind:
description: |-
Kind of the referent.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
name:
default: ""
description: |-
Name of the referent.
This field is effectively required, but due to backwards compatibility is
allowed to be empty. Instances of this type with an empty value here are
almost certainly wrong.
TODO: Add other useful fields. apiVersion, kind, uid?
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
TODO: Drop `kubebuilder:default` when controller-gen doesn't need it https://github.com/kubernetes-sigs/kubebuilder/issues/3896.
type: string
namespace:
description: |-
Namespace of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
type: string
resourceVersion:
description: |-
Specific resourceVersion to which this reference is made, if any.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
type: string
uid:
description: |-
UID of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
type: string
type: object
x-kubernetes-map-type: atomic
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/coreos/butane v0.21.0
github.com/go-logr/logr v1.4.2
github.com/ironcore-dev/controller-utils v0.9.3
github.com/ironcore-dev/metal v0.0.0-20240624081522-bfe4e65f0385
github.com/ironcore-dev/metal v0.0.0-20240624131301-18385f342755
github.com/onsi/ginkgo/v2 v2.19.0
github.com/onsi/gomega v1.33.1
k8s.io/api v0.30.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4=
github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY=
github.com/ironcore-dev/controller-utils v0.9.3 h1:sTrnxSzX5RrLf4B8KrAH2axSC+gxfJXphkV6df2GSsw=
github.com/ironcore-dev/controller-utils v0.9.3/go.mod h1:djKnxDs0Hwxhhc0VmVY8tZnrOrElvrRV2jov/LiCZ2Y=
github.com/ironcore-dev/metal v0.0.0-20240624081522-bfe4e65f0385 h1:I7lMRc6HSOUa2YCYP926uogP13U/FBLx5LOrYoa47iQ=
github.com/ironcore-dev/metal v0.0.0-20240624081522-bfe4e65f0385/go.mod h1:+/bmkghOE7acqXDT/LDH57RemaUzlVwnQjttsOjdoyg=
github.com/ironcore-dev/metal v0.0.0-20240624131301-18385f342755 h1:EmR3Ngg2wmOXJkxgsdYVuPXLRfwWmO2Fi+htjih6QGY=
github.com/ironcore-dev/metal v0.0.0-20240624131301-18385f342755/go.mod h1:+/bmkghOE7acqXDT/LDH57RemaUzlVwnQjttsOjdoyg=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
Expand Down
12 changes: 9 additions & 3 deletions internal/controller/httpbootconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (r *HTTPBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Lo
// Verify if the IgnitionRef is set, and it has the intended data key.
if HTTPBootConfig.Spec.IgnitionSecretRef != nil {
IgnitionSecret := &corev1.Secret{}
if err := r.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Namespace}, IgnitionSecret); err != nil {
if err := r.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Spec.IgnitionSecretRef.Namespace}, IgnitionSecret); err != nil {
return bootv1alpha1.HTTPBootConfigStateError, err
// TODO: Add some validation steps to ensure that the IgntionData is compliant, if necessary.
// Assume for now, that it's going to json format.
Expand Down Expand Up @@ -112,6 +112,10 @@ func (r *HTTPBootConfigReconciler) patchStatus(
HTTPBootConfig *bootv1alpha1.HTTPBootConfig,
state bootv1alpha1.HTTPBootConfigState,
) error {
if HTTPBootConfig.Status.State == state {
return nil
}

base := HTTPBootConfig.DeepCopy()
HTTPBootConfig.Status.State = state

Expand All @@ -130,14 +134,16 @@ func (r *HTTPBootConfigReconciler) enqueueHTTPBootConfigReferencingIgnitionSecre
}

list := &bootv1alpha1.HTTPBootConfigList{}
if err := r.Client.List(ctx, list, client.InNamespace(secretObj.Namespace)); err != nil {
if err := r.Client.List(ctx, list, client.InNamespace("")); err != nil {
log.Error(err, "failed to list HTTPBootConfig for secret", secret)
return nil
}

var requests []reconcile.Request
for _, HTTPBootConfig := range list.Items {
if HTTPBootConfig.Spec.IgnitionSecretRef != nil && HTTPBootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name {
if HTTPBootConfig.Spec.IgnitionSecretRef != nil &&
HTTPBootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name &&
HTTPBootConfig.Spec.IgnitionSecretRef.Namespace == secretObj.Namespace {
requests = append(requests, reconcile.Request{
NamespacedName: types.NamespacedName{
Name: HTTPBootConfig.Name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"

metalv1alpha1 "github.com/ironcore-dev/metal/api/v1alpha1"
)
Expand Down Expand Up @@ -90,7 +91,7 @@ func (r *MachineBootConfigurationHTTPReconciler) reconcile(ctx context.Context,
SystemUUID: systemUUID,
SystemIPs: systemIPs,
UKIURL: ukiURL,
IgnitionSecretRef: &corev1.LocalObjectReference{Name: config.Spec.IgnitionSecretRef.Name},
IgnitionSecretRef: &corev1.ObjectReference{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Spec.IgnitionSecretRef.Namespace},
},
}

Expand Down Expand Up @@ -179,5 +180,6 @@ func (r *MachineBootConfigurationHTTPReconciler) SetupWithManager(mgr ctrl.Manag
return ctrl.NewControllerManagedBy(mgr).
For(&metalv1alpha1.BootConfiguration{}).
Owns(&bootv1alpha1.HTTPBootConfig{}).
WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
Complete(r)
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (r *ServerBootConfigurationHTTPReconciler) reconcile(ctx context.Context, l
SystemUUID: systemUUID,
SystemIPs: systemIPs,
UKIURL: ukiURL,
IgnitionSecretRef: &corev1.LocalObjectReference{Name: config.Spec.IgnitionSecretRef.Name},
IgnitionSecretRef: &corev1.ObjectReference{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Namespace},
},
}

Expand Down
2 changes: 1 addition & 1 deletion server/bootserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func handleIgnitionHTTPBoot(w http.ResponseWriter, r *http.Request, k8sClient cl
HTTPBootConfig := HTTPBootConfigList.Items[0]

ignitionSecret := &corev1.Secret{}
if err := k8sClient.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Namespace}, ignitionSecret); err != nil {
if err := k8sClient.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Spec.IgnitionSecretRef.Namespace}, ignitionSecret); err != nil {
http.Error(w, "Resource Not Found", http.StatusNotFound)
log.Info("Error: Failed to get Ignition secret", "error", err.Error())
return
Expand Down

0 comments on commit d9873d3

Please sign in to comment.