Skip to content

Commit

Permalink
feat(securityGroups): Improve robustness of security group reconcilers
Browse files Browse the repository at this point in the history
  • Loading branch information
Guido van der Hart committed Sep 6, 2024
1 parent 20beff9 commit 8659d02
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 135 deletions.
3 changes: 3 additions & 0 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ type OscSecurityGroupRule struct {
// The ip range of the security group rule
// +optional
IpRange string `json:"ipRange,omitempty"`
// The name of the security group to use as target
// +optional
TargetSecurityGroupName string `json:"targetSecurityGroupName,omitempty"`
// The beginning of the port range
// +optional
FromPortRange int32 `json:"fromPortRange,omitempty"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.15.0
controller-gen.kubebuilder.io/version: v0.16.2
name: oscclusters.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down Expand Up @@ -364,6 +364,10 @@ spec:
resourceId:
description: The security group rule id
type: string
targetSecurityGroupName:
description: The name of the security group to use
as target
type: string
toPortRange:
description: The end of the port range
format: int32
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.15.0
controller-gen.kubebuilder.io/version: v0.16.2
name: oscclustertemplates.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down Expand Up @@ -49,26 +49,22 @@ spec:
ObjectMeta is metadata that all persisted resources must have, which includes all objects
users must create. This is a copy of customizable fields from metav1.ObjectMeta.
ObjectMeta is embedded in `Machine.Spec`, `MachineDeployment.Template` and `MachineSet.Template`,
which are not top-level Kubernetes objects. Given that metav1.ObjectMeta has lots of special cases
and read-only fields which end up in the generated CRD validation, having it as a subset simplifies
the API and some issues that can impact user experience.
During the [upgrade to controller-tools@v2](https://github.com/kubernetes-sigs/cluster-api/pull/1054)
for v1alpha2, we noticed a failure would occur running Cluster API test suite against the new CRDs,
specifically `spec.metadata.creationTimestamp in body must be of type string: "null"`.
The investigation showed that `controller-tools@v2` behaves differently than its previous version
when handling types from [metav1](k8s.io/apimachinery/pkg/apis/meta/v1) package.
In more details, we found that embedded (non-top level) types that embedded `metav1.ObjectMeta`
had validation properties, including for `creationTimestamp` (metav1.Time).
The `metav1.Time` type specifies a custom json marshaller that, when IsZero() is true, returns `null`
which breaks validation because the field isn't marked as nullable.
In future versions, controller-tools@v2 might allow overriding the type and validation for embedded
types. When that happens, this hack should be revisited.
properties:
Expand Down Expand Up @@ -436,6 +432,10 @@ spec:
resourceId:
description: The security group rule id
type: string
targetSecurityGroupName:
description: The name of the security group
to use as target
type: string
toPortRange:
description: The end of the port range
format: int32
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.15.0
controller-gen.kubebuilder.io/version: v0.16.2
name: oscmachines.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.15.0
controller-gen.kubebuilder.io/version: v0.16.2
name: oscmachinetemplates.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down Expand Up @@ -50,26 +50,22 @@ spec:
ObjectMeta is metadata that all persisted resources must have, which includes all objects
users must create. This is a copy of customizable fields from metav1.ObjectMeta.
ObjectMeta is embedded in `Machine.Spec`, `MachineDeployment.Template` and `MachineSet.Template`,
which are not top-level Kubernetes objects. Given that metav1.ObjectMeta has lots of special cases
and read-only fields which end up in the generated CRD validation, having it as a subset simplifies
the API and some issues that can impact user experience.
During the [upgrade to controller-tools@v2](https://github.com/kubernetes-sigs/cluster-api/pull/1054)
for v1alpha2, we noticed a failure would occur running Cluster API test suite against the new CRDs,
specifically `spec.metadata.creationTimestamp in body must be of type string: "null"`.
The investigation showed that `controller-tools@v2` behaves differently than its previous version
when handling types from [metav1](k8s.io/apimachinery/pkg/apis/meta/v1) package.
In more details, we found that embedded (non-top level) types that embedded `metav1.ObjectMeta`
had validation properties, including for `creationTimestamp` (metav1.Time).
The `metav1.Time` type specifies a custom json marshaller that, when IsZero() is true, returns `null`
which breaks validation because the field isn't marked as nullable.
In future versions, controller-tools@v2 might allow overriding the type and validation for embedded
types. When that happens, this hack should be revisited.
properties:
Expand Down
68 changes: 3 additions & 65 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,8 @@ rules:
- cluster.x-k8s.io
resources:
- clusters
verbs:
- get
- list
- watch
- apiGroups:
- cluster.x-k8s.io
resources:
- clusters/status
verbs:
- get
- list
- watch
- apiGroups:
- cluster.x-k8s.io
resources:
- machines
verbs:
- get
- list
- watch
- apiGroups:
- cluster.x-k8s.io
resources:
- machines/status
verbs:
- get
Expand All @@ -59,32 +38,8 @@ rules:
- infrastructure.cluster.x-k8s.io
resources:
- oscclusters
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscclusters/finalizers
verbs:
- update
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscclusters/status
verbs:
- get
- patch
- update
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscmachines
- oscmachinetemplates
verbs:
- create
- delete
Expand All @@ -96,32 +51,15 @@ rules:
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscclusters/finalizers
- oscmachines/finalizers
verbs:
- update
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscclusters/status
- oscmachines/status
verbs:
- get
- patch
- update
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscmachinetemplates
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
- oscmachinetemplates/status
verbs:
- get
Expand Down
Loading

0 comments on commit 8659d02

Please sign in to comment.