Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions api/v1alpha1/bmc_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,65 @@ type BMCSpec struct {
// Hostname is the hostname of the BMC.
// +optional
Hostname *string `json:"hostname,omitempty"`

// CertificateManagementPolicy controls automatic certificate management for this BMC.
// When not specified, the BMC inherits the operator-level default (configured via controller flags).
// Set to Manual to explicitly disable certificate management for this specific BMC.
// Set to Automatic to explicitly enable certificate management (overriding operator default if disabled).
//
// Certificate configuration (signer name, approval policy, renewal threshold, subject fields) is
// configured at the operator level via controller manager flags and cannot be overridden per-BMC.
// This ensures consistent certificate policy across all BMCs in the cluster.
//
// +optional
// +kubebuilder:validation:Enum=Manual;Automatic
CertificateManagementPolicy *CertificateManagementPolicy `json:"certificateManagementPolicy,omitempty"`
}

// CertificateManagementPolicy defines the policy for certificate management.
type CertificateManagementPolicy string

const (
// CertificateManagementPolicyManual means no automatic certificate operations
CertificateManagementPolicyManual CertificateManagementPolicy = "Manual"
// CertificateManagementPolicyAutomatic means automatic certificate creation and renewal
CertificateManagementPolicyAutomatic CertificateManagementPolicy = "Automatic"
)

// CertificateApprovalPolicy defines how CertificateSigningRequests are approved.
type CertificateApprovalPolicy string

const (
// CertificateApprovalPolicyAuto means the controller automatically approves CSRs.
// WARNING: Use only in trusted, isolated environments with verified BMC hardware.
// Not recommended for multi-tenant or untrusted environments.
CertificateApprovalPolicyAuto CertificateApprovalPolicy = "Auto"
// CertificateApprovalPolicyExternal means CSRs must be approved by external entity.
// Recommended for production environments. Requires cert-manager, admin, or custom approver.
CertificateApprovalPolicyExternal CertificateApprovalPolicy = "External"
)

// CertificateSubject defines certificate subject fields for CSR generation.
type CertificateSubject struct {
// Organization for the certificate subject.
// +optional
Organization string `json:"organization,omitempty"`

// OrganizationalUnit for the certificate subject.
// +optional
OrganizationalUnit string `json:"organizationalUnit,omitempty"`

// Country for the certificate subject.
// +optional
Country string `json:"country,omitempty"`

// State for the certificate subject.
// +optional
State string `json:"state,omitempty"`

// Locality (City) for the certificate subject.
// +optional
Locality string `json:"locality,omitempty"`
}

// InlineEndpoint defines inline network access configuration for the BMC.
Expand Down Expand Up @@ -219,6 +278,40 @@ type BMCStatus struct {
// +patchMergeKey=type
// +optional
Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`

// CertificateInfo contains information about the BMC's current certificate.
// +optional
CertificateInfo *CertificateInfo `json:"certificateInfo,omitempty"`

// CertificateSigningRequestRef references the current CertificateSigningRequest.
// +optional
CertificateSigningRequestRef *string `json:"certificateSigningRequestRef,omitempty"`

// CertificateSecretRef references the Secret containing the installed certificate.
// The Secret is created in the metal-operator controller's namespace.
// +optional
CertificateSecretRef *v1.LocalObjectReference `json:"certificateSecretRef,omitempty"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// CertificateInfo contains information about a BMC certificate.
type CertificateInfo struct {
// Issuer is the certificate issuer DN.
Issuer string `json:"issuer,omitempty"`

// Subject is the certificate subject DN.
Subject string `json:"subject,omitempty"`

// NotBefore is the certificate validity start time.
NotBefore *metav1.Time `json:"notBefore,omitempty"`

// NotAfter is the certificate validity end time.
NotAfter *metav1.Time `json:"notAfter,omitempty"`

// SerialNumber is the certificate serial number.
SerialNumber string `json:"serialNumber,omitempty"`

// Thumbprint is the SHA-256 thumbprint of the certificate.
Thumbprint string `json:"thumbprint,omitempty"`
}

// BMCState defines the possible states of a BMC.
Expand Down
58 changes: 58 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

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

85 changes: 85 additions & 0 deletions bmc/bmc.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ type BMC interface {
// CheckBMCPendingComponentUpgrade checks if there are pending/staged firmware upgrades
// for the given component type.
CheckBMCPendingComponentUpgrade(ctx context.Context, componentType ComponentType) (bool, error)

// GetCertificateService retrieves the certificate service for the BMC.
GetCertificateService(ctx context.Context) (*schemas.CertificateService, error)

// GenerateCSR generates a Certificate Signing Request on the BMC.
GenerateCSR(ctx context.Context, req CSRRequest) (*CSRResponse, error)

// InstallCertificate installs a certificate on the BMC.
InstallCertificate(ctx context.Context, certificatePEM string, certificateType CertificateType) error

// GetCertificates retrieves all installed certificates on the BMC.
GetCertificates(ctx context.Context) ([]CertificateInfo, error)

// DeleteCertificate deletes a certificate from the BMC.
DeleteCertificate(ctx context.Context, certificateURI string) error
}

type Entity struct {
Expand Down Expand Up @@ -300,3 +315,73 @@ type Manager struct {
MACAddress string
OemLinks json.RawMessage
}

// CertificateType defines the type of certificate.
type CertificateType string

const (
// CertificateTypeHTTPS is the HTTPS/TLS certificate type
CertificateTypeHTTPS CertificateType = "PEM"
// CertificateTypeCA is the CA certificate type
CertificateTypeCA CertificateType = "PEM"
)
Comment on lines +322 to +327
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Both certificate type constants share the same value "PEM" — filtering by type is broken.

CertificateTypeHTTPS and CertificateTypeCA are defined with the identical underlying value "PEM", so any consumer that distinguishes them (e.g. the controller's cert.Type == bmc.CertificateTypeHTTPS check in updateCertificateInfo) will treat CA and HTTPS certs as indistinguishable. Note also that Redfish's CertificateType property enumerates encoding formats (PEM, PEMchain, PKCS7) rather than purpose — if these constants are meant to express certificate purpose, they should be modeled separately from the Redfish wire value (and likely derived from the certificate's collection URI).

🐛 Minimal fix to differentiate the two values
 const (
 	// CertificateTypeHTTPS is the HTTPS/TLS certificate type
 	CertificateTypeHTTPS CertificateType = "PEM"
 	// CertificateTypeCA is the CA certificate type
-	CertificateTypeCA CertificateType = "PEM"
+	CertificateTypeCA CertificateType = "CA"
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const (
// CertificateTypeHTTPS is the HTTPS/TLS certificate type
CertificateTypeHTTPS CertificateType = "PEM"
// CertificateTypeCA is the CA certificate type
CertificateTypeCA CertificateType = "PEM"
)
const (
// CertificateTypeHTTPS is the HTTPS/TLS certificate type
CertificateTypeHTTPS CertificateType = "PEM"
// CertificateTypeCA is the CA certificate type
CertificateTypeCA CertificateType = "CA"
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bmc/bmc.go` around lines 319 - 324, CertificateTypeHTTPS and
CertificateTypeCA are both set to the same underlying value "PEM", causing code
that checks cert.Type (e.g., updateCertificateInfo's cert.Type ==
bmc.CertificateTypeHTTPS) to be unable to distinguish HTTPS vs CA certificates;
fix by giving these purpose constants distinct values (e.g., "HTTPS" and "CA")
or by separating purpose vs Redfish encoding (introduce separate enums/constants
for CertificatePurpose vs CertificateEncoding and update usages such as
CertificateTypeHTTPS, CertificateTypeCA and any code reading Redfish's
CertificateType to reference the encoding constants instead).


// CertificatePurpose defines the purpose/usage of a certificate.
type CertificatePurpose string

const (
// CertificatePurposeHTTPS indicates an HTTPS/TLS server certificate
CertificatePurposeHTTPS CertificatePurpose = "HTTPS"
// CertificatePurposeCA indicates a CA certificate
CertificatePurposeCA CertificatePurpose = "CA"
)

// CSRRequest contains parameters for generating a Certificate Signing Request.
type CSRRequest struct {
// CommonName is the common name for the certificate (typically hostname or IP)
CommonName string
// Organization is the organization name
Organization string
// OrganizationalUnit is the organizational unit
OrganizationalUnit string
// Country is the two-letter country code
Country string
// State is the state or province
State string
// City is the city or locality
City string
// Email is the email address
Email string
// KeyPairAlgorithm is the key pair algorithm (e.g., "RSA2048", "RSA4096", "EC256")
KeyPairAlgorithm string
// AlternativeNames are subject alternative names (DNS names, IP addresses)
AlternativeNames []string
}

// CSRResponse contains the generated Certificate Signing Request.
type CSRResponse struct {
// CSRString is the PEM-encoded PKCS#10 CSR
CSRString string
// CertificateCollection is the URI where the signed certificate should be installed
CertificateCollection string
}

// CertificateInfo contains information about an installed certificate.
type CertificateInfo struct {
// URI is the Redfish URI of the certificate resource
URI string
// Type is the certificate type
Type CertificateType
// Issuer is the certificate issuer distinguished name
Issuer string
// Subject is the certificate subject distinguished name
Subject string
// ValidNotBefore is the certificate validity start time
ValidNotBefore string
// ValidNotAfter is the certificate validity end time
ValidNotAfter string
// SerialNumber is the certificate serial number
SerialNumber string
// Fingerprint is the certificate fingerprint/thumbprint
Fingerprint string
}
Loading
Loading