Skip to content

Commit

Permalink
Rework naming of references in the API
Browse files Browse the repository at this point in the history 
As discussed in
#91, lets
use simpler reference names and reserve the SomethingRef format for when
the field can refer to different kinds or at least namespaces.

This is quite a breaking change but I don't see a lot of value in going
through deprecation since it's going to significantly complicate the
code nobody should be using in production yet.

Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com>
  • Loading branch information
@dtantsur
dtantsur committed Nov 28, 2024
1 parent de02d4a commit c921bf0
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 166 deletions.
14 changes: 6 additions & 8 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ metadata:
spec:
deployRamdisk:
sshKey: "<YOUR SSH PUBLIC KEY HERE>"
tlsRef:
name: ironic-tls
tls:
certificateName: ironic-tls
```
```bash
Expand Down Expand Up @@ -121,19 +121,17 @@ metadata:
name: ironic
namespace: test
spec:
tlsRef:
name: ironic-tls
tlsCertificateName: ironic-tls
---
apiVersion: metal3.io/v1alpha1
kind: Ironic
metadata:
name: ironic
namespace: test
spec:
databaseRef:
name: ironic
tlsRef:
name: ironic-tls
databaseName: ironic
tls:
certificateName: ironic-tls
networking:
ipAddress: 10.89.0.2
dhcp:
Expand Down
42 changes: 24 additions & 18 deletions api/v1alpha1/ironic_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package v1alpha1

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -156,21 +155,11 @@ type DeployRamdisk struct {
SSHKey string `json:"sshKey,omitempty"`
}

// IronicSpec defines the desired state of Ironic
type IronicSpec struct {
// CredentialsRef is a reference to the secret with Ironic API credentials.
// A new secret will be created if this field is empty.
// +optional
CredentialsRef corev1.LocalObjectReference `json:"credentialsRef,omitempty"`

// DatabaseRef defines database settings for Ironic.
// If missing, a local SQLite database will be used. Must be provided for a highly available architecture.
// TLS defines the TLS settings.
type TLS struct {
// CertificateName is a reference to the secret with the TLS certificate.
// +optional
DatabaseRef corev1.LocalObjectReference `json:"databaseRef,omitempty"`

// DeployRamdisk defines settings for the provisioning/inspection ramdisk based on Ironic Python Agent.
// +optional
DeployRamdisk DeployRamdisk `json:"deployRamdisk,omitempty"`
CertificateName string `json:"certificateName,omitempty"`

// DisableVirtualMediaTLS turns off TLS on the virtual media server,
// which may be required for hardware that cannot accept HTTPS links.
Expand All @@ -182,9 +171,26 @@ type IronicSpec struct {
// Has no effect if HighAvailability is not set to true.
// +optional
DisableRPCHostValidation bool `json:"disableRPCHostValidation,omitempty"`
}

// IronicSpec defines the desired state of Ironic
type IronicSpec struct {
// APICredentialsName is a reference to the secret with Ironic API credentials.
// A new secret will be created if this field is empty.
// +optional
APICredentialsName string `json:"apiCredentialsName,omitempty"`

// DatabaseName is a reference to the IronicDatabase object.
// If missing, a local SQLite database will be used. Must be provided for a highly available architecture.
// +optional
DatabaseName string `json:"databaseName,omitempty"`

// DeployRamdisk defines settings for the provisioning/inspection ramdisk based on Ironic Python Agent.
// +optional
DeployRamdisk DeployRamdisk `json:"deployRamdisk,omitempty"`

// HighAvailability causes Ironic to be deployed as a DaemonSet on control plane nodes instead of a deployment with 1 replica.
// Requires database to be installed and linked to DatabaseRef.
// Requires database to be installed and linked to DatabaseName.
// EXPERIMENTAL: do not use (validation will fail)!
// +optional
HighAvailability bool `json:"highAvailability,omitempty"`
Expand All @@ -202,9 +208,9 @@ type IronicSpec struct {
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`

// TLSRef is a reference to the secret with the database TLS certificate.
// TLS defines TLS-related settings for various network interactions.
// +optional
TLSRef corev1.LocalObjectReference `json:"tlsRef,omitempty"`
TLS TLS `json:"tls,omitempty"`
}

// InstalledVersion identifies which version of Ironic was installed.
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/ironic_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ func ValidateDHCP(ironic *IronicSpec, dhcp *DHCP) error {
}

func ValidateIronic(ironic *IronicSpec, old *IronicSpec) error {
if ironic.HighAvailability && ironic.DatabaseRef.Name == "" {
if ironic.HighAvailability && ironic.DatabaseName == "" {
return errors.New("database is required for highly available architecture")
}

if old != nil && old.DatabaseRef.Name != "" && old.DatabaseRef.Name != ironic.DatabaseRef.Name {
if old != nil && old.DatabaseName != "" && old.DatabaseName != ironic.DatabaseName {
return errors.New("cannot change to a new database or remove it")
}

Expand Down
4 changes: 1 addition & 3 deletions api/v1alpha1/ironic_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ func TestValidateIronic(t *testing.T) {
{
Scenario: "with database",
Ironic: IronicSpec{
DatabaseRef: corev1.LocalObjectReference{
Name: "db",
},
DatabaseName: "db",
},
},
{
Expand Down
9 changes: 4 additions & 5 deletions api/v1alpha1/ironicdatabase_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,25 @@ limitations under the License.
package v1alpha1

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// IronicDatabaseSpec defines the desired state of IronicDatabase
type IronicDatabaseSpec struct {
// CredentialsSecretRef is a reference to the secret with database credentials.
// CredentialsName is a reference to the secret with database credentials.
// A new secret will be created if this field is empty.
// +optional
CredentialsRef corev1.LocalObjectReference `json:"credentialsRef,omitempty"`
CredentialsName string `json:"credentialsName,omitempty"`

// Image is the MariaDB image.
// +kubebuilder:default=quay.io/metal3-io/mariadb
// +kubebuilder:validation:MinLength=1
// +optional
Image string `json:"image,omitempty"`

// TLSSecretName is a reference to the secret with the database TLS certificate.
// TLSCertificateName is a reference to the secret with the database TLS certificate.
// +optional
TLSRef corev1.LocalObjectReference `json:"tlsRef,omitempty"`
TLSCertificateName string `json:"tlsCertificateName,omitempty"`
}

// IronicDatabaseStatus defines the observed state of IronicDatabase
Expand Down
21 changes: 16 additions & 5 deletions api/v1alpha1/zz_generated.deepcopy.go
View file

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

40 changes: 7 additions & 33 deletions config/crd/bases/metal3.io_ironicdatabases.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,46 +39,20 @@ spec:
spec:
description: IronicDatabaseSpec defines the desired state of IronicDatabase
properties:
credentialsRef:
credentialsName:
description: |-
CredentialsSecretRef is a reference to the secret with database credentials.
CredentialsName is a reference to the secret with database credentials.
A new secret will be created if this field is empty.
properties:
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
type: object
x-kubernetes-map-type: atomic
type: string
image:
default: quay.io/metal3-io/mariadb
description: Image is the MariaDB image.
minLength: 1
type: string
tlsRef:
description: TLSSecretName is a reference to the secret with the database
TLS certificate.
properties:
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
type: object
x-kubernetes-map-type: atomic
tlsCertificateName:
description: TLSCertificateName is a reference to the secret with
the database TLS certificate.
type: string
type: object
status:
description: IronicDatabaseStatus defines the observed state of IronicDatabase
Expand Down
82 changes: 24 additions & 58 deletions config/crd/bases/metal3.io_ironics.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,42 +39,16 @@ spec:
spec:
description: IronicSpec defines the desired state of Ironic
properties:
credentialsRef:
apiCredentialsName:
description: |-
CredentialsRef is a reference to the secret with Ironic API credentials.
APICredentialsName is a reference to the secret with Ironic API credentials.
A new secret will be created if this field is empty.
properties:
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
type: object
x-kubernetes-map-type: atomic
databaseRef:
type: string
databaseName:
description: |-
DatabaseRef defines database settings for Ironic.
DatabaseName is a reference to the IronicDatabase object.
If missing, a local SQLite database will be used. Must be provided for a highly available architecture.
properties:
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
type: object
x-kubernetes-map-type: atomic
type: string
deployRamdisk:
description: DeployRamdisk defines settings for the provisioning/inspection
ramdisk based on Ironic Python Agent.
Expand All @@ -94,21 +68,10 @@ spec:
into the ramdisk for debugging purposes.
type: string
type: object
disableRPCHostValidation:
description: |-
DisableRPCHostValidation turns off TLS host validation for JSON RPC connections between Ironic instances.
This reduces the security of TLS. Only use if you're unable to provide TLS certificates valid for JSON RPC.
Has no effect if HighAvailability is not set to true.
type: boolean
disableVirtualMediaTLS:
description: |-
DisableVirtualMediaTLS turns off TLS on the virtual media server,
which may be required for hardware that cannot accept HTTPS links.
type: boolean
highAvailability:
description: |-
HighAvailability causes Ironic to be deployed as a DaemonSet on control plane nodes instead of a deployment with 1 replica.
Requires database to be installed and linked to DatabaseRef.
Requires database to be installed and linked to DatabaseName.
EXPERIMENTAL: do not use (validation will fail)!
type: boolean
inspection:
Expand Down Expand Up @@ -247,23 +210,26 @@ spec:
Selector which must match a node's labels for the vmi to be scheduled on that node.
More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
type: object
tlsRef:
description: TLSRef is a reference to the secret with the database
TLS certificate.
tls:
description: TLS defines TLS-related settings for various network
interactions.
properties:
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.
certificateName:
description: CertificateName is a reference to the secret with
the TLS certificate.
type: string
disableRPCHostValidation:
description: |-
DisableRPCHostValidation turns off TLS host validation for JSON RPC connections between Ironic instances.
This reduces the security of TLS. Only use if you're unable to provide TLS certificates valid for JSON RPC.
Has no effect if HighAvailability is not set to true.
type: boolean
disableVirtualMediaTLS:
description: |-
DisableVirtualMediaTLS turns off TLS on the virtual media server,
which may be required for hardware that cannot accept HTTPS links.
type: boolean
type: object
x-kubernetes-map-type: atomic
type: object
status:
description: IronicStatus defines the observed state of Ironic
Expand Down
Loading

0 comments on commit c921bf0

Please sign in to comment.