-
Notifications
You must be signed in to change notification settings - Fork 1.4k
✨ Enable ssatags KAL linter #12470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Enable ssatags KAL linter #12470
Changes from all commits
0463e8e
f5ac2b2
a90c31f
18f7f63
8fc263a
19c90e8
596994a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,6 +64,7 @@ type ClusterResourceSetSpec struct { | |
|
||
// resources is a list of Secrets/ConfigMaps where each contains 1 or more resources to be applied to remote clusters. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This field is a classic example of one that would probably be listTypeMap. Each name/kind pair within the list should be unique right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be unique, yes. But I think that doesn't justify making this field a listType=map:
If we want that validation we should achieve it through other means that don't lead to these issues. In my opinion we should be very careful with listType=map and should only use it:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JoelSpeed Let's discuss this here first. This has impact on almost all other findings. I'm really not in favor of making arrays maps if we can come up with a key for the reasons mentioned above. I'm not opposed to adding additional validation through other means, but honestly I won't have the bandwith for it in this release cycle. If someone wants to open a PR to add CEL validation with appropriate envtest-based unit test coverage that's fine for me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, when adding new fields I've always recommended, where there is an obvious map key (list of named objects style) to go map over atomic. In general those decisions have aged well so far to my knowledge and we haven't hit any issues with significant memory usage. You are right about changing things day 2 though, the SSA migration from atomic to map has had issues in the past, but I was under the impression some fixes went in to resolve those migration issues? When we've had issues in the past with changing the atomic->map post release, was that during a migration between API versions? Or within the same API version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It was within the same apiVersion. IIRC I also tested it at the time and the same thing happens even between apiVersions. (and yes there were some fixes in the structured merge library but they were years ago, or at least I'm not aware of others) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see this comment: #11857 (comment) Specifically:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding my two cents on why we should not rush in transforming atomic list on map lists:
|
||
// +kubebuilder:validation:MaxItems=100 | ||
Resources []ResourceRef `json:"resources,omitempty"` | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,7 @@ type ResourceSetBinding struct { | |
|
||
// resources is a list of resources that the ClusterResourceSet has. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// +kubebuilder:validation:MaxItems=100 | ||
Resources []ResourceBinding `json:"resources,omitempty"` | ||
} | ||
|
@@ -137,6 +138,7 @@ type ClusterResourceSetBinding struct { | |
type ClusterResourceSetBindingSpec struct { | ||
// bindings is a list of ClusterResourceSets and their resources. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
Bindings []*ResourceSetBinding `json:"bindings,omitempty"` | ||
sbueringer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,6 +77,7 @@ type InitConfiguration struct { | |
// bootstrapTokens is respected at `kubeadm init` time and describes a set of Bootstrap Tokens to create. | ||
// This information IS NOT uploaded to the kubeadm cluster configmap, partly because of its sensitive nature | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each |
||
// +kubebuilder:validation:MaxItems=100 | ||
BootstrapTokens []BootstrapToken `json:"bootstrapTokens,omitempty"` | ||
|
||
|
@@ -99,6 +100,7 @@ type InitConfiguration struct { | |
// The list of phases can be obtained with the "kubeadm init --help" command. | ||
// This option takes effect only on Kubernetes >=1.22.0. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=50 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=256 | ||
|
@@ -199,18 +201,21 @@ type APIServer struct { | |
|
||
// extraVolumes is an extra set of host volumes, mounted to the control plane component. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a clear map key on this apart from |
||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraVolumes []HostPathMount `json:"extraVolumes,omitempty"` | ||
|
||
// extraEnvs is an extra set of environment variables to pass to the control plane component. | ||
// Environment variables passed using ExtraEnvs will override any existing environment variables, or *_proxy environment variables that kubeadm adds by default. | ||
// This option takes effect only on Kubernetes >=1.31.0. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Map with the env var name as the key? |
||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraEnvs []EnvVar `json:"extraEnvs,omitempty"` | ||
|
||
// certSANs sets extra Subject Alternative Names for the API Server signing cert. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=253 | ||
|
@@ -234,13 +239,15 @@ type ControllerManager struct { | |
|
||
// extraVolumes is an extra set of host volumes, mounted to the control plane component. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraVolumes []HostPathMount `json:"extraVolumes,omitempty"` | ||
|
||
// extraEnvs is an extra set of environment variables to pass to the control plane component. | ||
// Environment variables passed using ExtraEnvs will override any existing environment variables, or *_proxy environment variables that kubeadm adds by default. | ||
// This option takes effect only on Kubernetes >=1.31.0. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraEnvs []EnvVar `json:"extraEnvs,omitempty"` | ||
} | ||
|
@@ -262,13 +269,15 @@ type Scheduler struct { | |
|
||
// extraVolumes is an extra set of host volumes, mounted to the control plane component. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraVolumes []HostPathMount `json:"extraVolumes,omitempty"` | ||
|
||
// extraEnvs is an extra set of environment variables to pass to the control plane component. | ||
// Environment variables passed using ExtraEnvs will override any existing environment variables, or *_proxy environment variables that kubeadm adds by default. | ||
// This option takes effect only on Kubernetes >=1.31.0. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraEnvs []EnvVar `json:"extraEnvs,omitempty"` | ||
} | ||
|
@@ -357,6 +366,7 @@ type NodeRegistrationOptions struct { | |
// ignorePreflightErrors provides a slice of pre-flight errors to be ignored when the current node is registered, e.g. 'IsPrivilegedUser,Swap'. | ||
// Value 'all' ignores errors from all checks. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=50 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=512 | ||
|
@@ -383,31 +393,38 @@ type BootstrapToken struct { | |
// Used for joining nodes in the cluster. | ||
// +required | ||
Token *BootstrapTokenString `json:"token"` | ||
|
||
// description sets a human-friendly message why this token exists and what it's used | ||
// for, so other administrators can know its purpose. | ||
// +optional | ||
// +kubebuilder:validation:MinLength=1 | ||
// +kubebuilder:validation:MaxLength=512 | ||
Description string `json:"description,omitempty"` | ||
|
||
// ttlSeconds defines the time to live for this token. Defaults to 24h. | ||
// Expires and ttlSeconds are mutually exclusive. | ||
// +optional | ||
// +kubebuilder:validation:Minimum=0 | ||
TTLSeconds *int32 `json:"ttlSeconds,omitempty"` | ||
|
||
// expires specifies the timestamp when this token expires. Defaults to being set | ||
// dynamically at runtime based on the ttlSeconds. Expires and ttlSeconds are mutually exclusive. | ||
// +optional | ||
Expires *metav1.Time `json:"expires,omitempty"` | ||
|
||
// usages describes the ways in which this token can be used. Can by default be used | ||
// for establishing bidirectional trust, but that can be changed here. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=256 | ||
Usages []string `json:"usages,omitempty"` | ||
|
||
// groups specifies the extra groups that this token will authenticate as when/if | ||
// used for authentication | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=256 | ||
|
@@ -456,18 +473,21 @@ type LocalEtcd struct { | |
// Environment variables passed using ExtraEnvs will override any existing environment variables, or *_proxy environment variables that kubeadm adds by default. | ||
// This option takes effect only on Kubernetes >=1.31.0. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
ExtraEnvs []EnvVar `json:"extraEnvs,omitempty"` | ||
|
||
// serverCertSANs sets extra Subject Alternative Names for the etcd server signing cert. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=253 | ||
ServerCertSANs []string `json:"serverCertSANs,omitempty"` | ||
|
||
// peerCertSANs sets extra Subject Alternative Names for the etcd peer signing cert. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=253 | ||
|
@@ -479,6 +499,7 @@ type LocalEtcd struct { | |
type ExternalEtcd struct { | ||
// endpoints of etcd members. Required for ExternalEtcd. | ||
// +required | ||
// +listType=atomic | ||
// +kubebuilder:validation:MinItems=1 | ||
// +kubebuilder:validation:MaxItems=50 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
|
@@ -538,6 +559,7 @@ type JoinConfiguration struct { | |
// The list of phases can be obtained with the "kubeadm init --help" command. | ||
// This option takes effect only on Kubernetes >=1.22.0. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=50 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=256 | ||
|
@@ -606,6 +628,7 @@ type BootstrapTokenDiscovery struct { | |
// ASN.1. These hashes can be calculated using, for example, OpenSSL: | ||
// openssl x509 -pubkey -in ca.crt openssl rsa -pubin -outform der 2>&/dev/null | openssl dgst -sha256 -hex | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=512 | ||
|
@@ -743,6 +766,7 @@ type KubeConfigAuthExec struct { | |
|
||
// args is the arguments to pass to the command when executing it. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=512 | ||
|
@@ -752,6 +776,7 @@ type KubeConfigAuthExec struct { | |
// are unioned with the host's environment, as well as variables client-go uses | ||
// to pass argument to the plugin. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
Env []KubeConfigAuthExecEnv `json:"env,omitempty"` | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,7 @@ type KubeadmConfigSpec struct { | |
|
||
// files specifies extra files to be passed to user_data upon creation. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be map on the path? |
||
// +kubebuilder:validation:MaxItems=200 | ||
Files []File `json:"files,omitempty"` | ||
|
||
|
@@ -75,13 +76,15 @@ type KubeadmConfigSpec struct { | |
|
||
// mounts specifies a list of mount points to be setup. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
Mounts []MountPoints `json:"mounts,omitempty"` | ||
|
||
// bootCommands specifies extra commands to run very early in the boot process via the cloud-init bootcmd | ||
// module. bootcmd will run on every boot, 'cloud-init-per' command can be used to make bootcmd run exactly | ||
// once. This is typically run in the cloud-init.service systemd unit. This has no effect in Ignition. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=1000 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=10240 | ||
|
@@ -91,6 +94,7 @@ type KubeadmConfigSpec struct { | |
// With cloud-init, this is prepended to the runcmd module configuration, and is typically executed in | ||
// the cloud-final.service systemd unit. In Ignition, this is prepended to /etc/kubeadm.sh. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=1000 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=10240 | ||
|
@@ -100,13 +104,15 @@ type KubeadmConfigSpec struct { | |
// With cloud-init, this is appended to the runcmd module configuration, and is typically executed in | ||
// the cloud-final.service systemd unit. In Ignition, this is appended to /etc/kubeadm.sh. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=1000 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=10240 | ||
PostKubeadmCommands []string `json:"postKubeadmCommands,omitempty"` | ||
|
||
// users specifies extra users to add | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Map on name? |
||
// +kubebuilder:validation:MaxItems=100 | ||
Users []User `json:"users,omitempty"` | ||
|
||
|
@@ -766,6 +772,7 @@ type User struct { | |
|
||
// sshAuthorizedKeys specifies a list of ssh authorized keys for the user | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=2048 | ||
|
@@ -776,6 +783,7 @@ type User struct { | |
type NTP struct { | ||
// servers specifies which NTP servers to use | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=512 | ||
|
@@ -790,11 +798,13 @@ type NTP struct { | |
type DiskSetup struct { | ||
// partitions specifies the list of the partitions to setup. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Map on device? |
||
// +kubebuilder:validation:MaxItems=100 | ||
Partitions []Partition `json:"partitions,omitempty"` | ||
|
||
// filesystems specifies the list of file systems to setup. | ||
// +optional | ||
// +listType=atomic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Map on device? |
||
// +kubebuilder:validation:MaxItems=100 | ||
Filesystems []Filesystem `json:"filesystems,omitempty"` | ||
} | ||
|
@@ -863,6 +873,7 @@ type Filesystem struct { | |
|
||
// extraOpts defined extra options to add to the command for creating the file system. | ||
// +optional | ||
// +listType=atomic | ||
// +kubebuilder:validation:MaxItems=100 | ||
// +kubebuilder:validation:items:MinLength=1 | ||
// +kubebuilder:validation:items:MaxLength=256 | ||
|
Uh oh!
There was an error while loading. Please reload this page.