Skip to content

Improve Redfish protocol and TLS certificate verification handling #739

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.

Sign up for GitHub

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

Merged
afritzler merged 3 commits into from
Apr 2, 2026
Merged

Improve Redfish protocol and TLS certificate verification handling #739

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b19bfd
Fix Redfish protocol and TLS certificate verification handling
stefanhipfel Mar 17, 2026
d19957c
Merge branch 'main' into fix/issue-395-tls-certificate-validation
stefanhipfel Mar 30, 2026
dd83b8d
Merge main and fix CodeRabbit issues
stefanhipfel Apr 1, 2026
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
5 changes: 4 additions & 1 deletion bmc/redfish.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ type Options struct {
Password string
BasicAuth bool

// TLS configuration
InsecureTLS bool // Skip TLS certificate verification

ResourcePollingInterval time.Duration
ResourcePollingTimeout time.Duration
PowerPollingInterval time.Duration
Expand Down Expand Up @@ -87,7 +90,7 @@ func newRedfishBaseBMCClient(ctx context.Context, options Options) (*RedfishBase
Endpoint: options.Endpoint,
Username: options.Username,
Password: options.Password,
Insecure: true,
Insecure: options.InsecureTLS,
BasicAuth: options.BasicAuth,
}
client, err := gofish.ConnectContext(ctx, clientConfig)
Expand Down
117 changes: 83 additions & 34 deletions cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ func main() { // nolint: gocyclo
enableHTTP2 bool
macPrefixesFile string
insecure bool
protocol string
skipCertValidation bool
managerNamespace string
probeImage string
probeOSImage string
Expand Down Expand Up @@ -136,7 +138,11 @@ func main() { // nolint: gocyclo
flag.StringVar(&probeImage, "probe-image", "", "Image for the first boot probing of a Server.")
flag.StringVar(&probeOSImage, "probe-os-image", "", "OS image for the first boot probing of a Server.")
flag.StringVar(&managerNamespace, "manager-namespace", "default", "Namespace the manager is running in.")
flag.BoolVar(&insecure, "insecure", true, "If true, use http instead of https for connecting to a BMC.")
flag.BoolVar(&insecure, "insecure", true, "Deprecated: Use --protocol and --skip-cert-validation instead")
flag.StringVar(&protocol, "protocol", "",
"Protocol to use for BMC connections: 'http' or 'https'. "+
"If not set, defaults based on --insecure flag for compatibility")
flag.BoolVar(&skipCertValidation, "skip-cert-validation", false, "Skip TLS certificate validation when using HTTPS")
flag.StringVar(&macPrefixesFile, "mac-prefixes-file", "", "Location of the MAC prefixes file.")
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
Expand Down Expand Up @@ -172,6 +178,41 @@ func main() { // nolint: gocyclo

ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

// Handle protocol and TLS certificate validation flags with backward compatibility
var effectiveProtocol metalv1alpha1.ProtocolScheme
var effectiveSkipCert bool

if protocol != "" {
// New flag takes precedence
switch protocol {
case "http":
effectiveProtocol = metalv1alpha1.HTTPProtocolScheme
case "https":
effectiveProtocol = metalv1alpha1.HTTPSProtocolScheme
default:
setupLog.Error(nil, "Invalid protocol value. Must be 'http' or 'https'", "protocol", protocol)
os.Exit(1)
}
effectiveSkipCert = skipCertValidation
} else {
// Backward compatibility: use insecure flag
if insecure {
effectiveProtocol = metalv1alpha1.HTTPProtocolScheme
effectiveSkipCert = true // HTTP doesn't use TLS anyway
setupLog.Info("Using deprecated --insecure flag. Please migrate to --protocol=http")
} else {
effectiveProtocol = metalv1alpha1.HTTPSProtocolScheme
// Legacy behavior: insecure=false still skipped cert validation
effectiveSkipCert = true
setupLog.Info("Using deprecated --insecure=false flag. " +
"Please migrate to --protocol=https --skip-cert-validation=false for secure connections")
}
}

if effectiveSkipCert && effectiveProtocol == metalv1alpha1.HTTPSProtocolScheme {
setupLog.Info("WARNING: TLS certificate verification is disabled. This is not recommended for production")
}

if probeOSImage == "" {
setupLog.Error(nil, "probe OS image must be set")
os.Exit(1)
Expand Down Expand Up @@ -351,10 +392,11 @@ func main() { // nolint: gocyclo
setupLog.Info("Registered custom server metrics collector")

if err = (&controller.EndpointReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
MACPrefixes: macPRefixes,
Insecure: insecure,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
MACPrefixes: macPRefixes,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "Failed to create controller", "controller", "Endpoint")
os.Exit(1)
Expand All @@ -369,7 +411,8 @@ func main() { // nolint: gocyclo
if err = (&controller.BMCReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Insecure: insecure,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
BMCFailureResetDelay: bmcFailureResetDelay,
BMCResetWaitTime: bmcResetWaitingInterval,
BMCClientRetryInterval: bmcResetResyncInterval,
Expand All @@ -387,7 +430,8 @@ func main() { // nolint: gocyclo
if err = (&controller.ServerReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Insecure: insecure,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
ManagerNamespace: managerNamespace,
ProbeImage: probeImage,
ProbeOSImage: probeOSImage,
Expand Down Expand Up @@ -435,12 +479,13 @@ func main() { // nolint: gocyclo
os.Exit(1)
}
if err = (&controller.BIOSSettingsReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
Insecure: insecure,
ResyncInterval: maintenanceResyncInterval,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
ResyncInterval: maintenanceResyncInterval,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
BMCOptions: bmc.Options{
BasicAuth: true,
PowerPollingInterval: powerPollingInterval,
Expand All @@ -454,12 +499,13 @@ func main() { // nolint: gocyclo
os.Exit(1)
}
if err = (&controller.BIOSVersionReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
Insecure: insecure,
ResyncInterval: maintenanceResyncInterval,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
ResyncInterval: maintenanceResyncInterval,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
BMCOptions: bmc.Options{
BasicAuth: true,
PowerPollingInterval: powerPollingInterval,
Expand All @@ -472,12 +518,13 @@ func main() { // nolint: gocyclo
os.Exit(1)
}
if err = (&controller.BMCSettingsReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
ResyncInterval: maintenanceResyncInterval,
Insecure: insecure,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
ResyncInterval: maintenanceResyncInterval,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
BMCOptions: bmc.Options{
BasicAuth: true,
PowerPollingInterval: powerPollingInterval,
Expand All @@ -490,12 +537,13 @@ func main() { // nolint: gocyclo
os.Exit(1)
}
if err = (&controller.BMCVersionReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
Insecure: insecure,
ResyncInterval: maintenanceResyncInterval,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ManagerNamespace: managerNamespace,
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
ResyncInterval: maintenanceResyncInterval,
Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
BMCOptions: bmc.Options{
BasicAuth: true,
PowerPollingInterval: powerPollingInterval,
Expand Down Expand Up @@ -540,9 +588,10 @@ func main() { // nolint: gocyclo
os.Exit(1)
}
if err = (&controller.BMCUserReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Insecure: insecure,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
DefaultProtocol: effectiveProtocol,
SkipCertValidation: effectiveSkipCert,
BMCOptions: bmc.Options{
BasicAuth: true,
PowerPollingInterval: powerPollingInterval,
Expand Down
22 changes: 11 additions & 11 deletions internal/bmcutils/bmcutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,11 @@ const (
BMCConnectivityCheckOption BMCClientOptions = 1
)

func GetProtocolScheme(scheme metalv1alpha1.ProtocolScheme, insecure bool) metalv1alpha1.ProtocolScheme {
func GetProtocolScheme(scheme metalv1alpha1.ProtocolScheme, defaultScheme metalv1alpha1.ProtocolScheme) metalv1alpha1.ProtocolScheme {
if scheme != "" {
return scheme
}
if insecure {
return metalv1alpha1.HTTPProtocolScheme
}
return metalv1alpha1.HTTPSProtocolScheme
return defaultScheme
}

func GetBMCCredentialsFromSecret(secret *metalv1alpha1.BMCSecret) (string, string, error) {
Expand Down Expand Up @@ -110,15 +107,15 @@ func GetBMCAddressForBMC(ctx context.Context, c client.Client, bmcObj *metalv1al

const DefaultKubeNamespace = "default"

func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1alpha1.Server, insecure bool, options bmc.Options) (bmc.BMC, error) {
func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1alpha1.Server, defaultProtocol metalv1alpha1.ProtocolScheme, skipCertValidation bool, options bmc.Options) (bmc.BMC, error) {
if server.Spec.BMCRef != nil {
b := &metalv1alpha1.BMC{}
bmcName := server.Spec.BMCRef.Name
if err := c.Get(ctx, client.ObjectKey{Name: bmcName}, b); err != nil {
return nil, fmt.Errorf("failed to get BMC: %w", err)
}

return GetBMCClientFromBMC(ctx, c, b, insecure, options)
return GetBMCClientFromBMC(ctx, c, b, defaultProtocol, skipCertValidation, options)
}

if server.Spec.BMC != nil {
Expand All @@ -127,7 +124,7 @@ func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1
return nil, fmt.Errorf("failed to get BMC secret: %w", err)
}

protocolScheme := GetProtocolScheme(server.Spec.BMC.Protocol.Scheme, insecure)
protocolScheme := GetProtocolScheme(server.Spec.BMC.Protocol.Scheme, defaultProtocol)

return CreateBMCClient(
ctx,
Expand All @@ -138,13 +135,14 @@ func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1
server.Spec.BMC.Protocol.Port,
bmcSecret,
options,
skipCertValidation,
)
}

return nil, fmt.Errorf("server %s has neither a BMCRef nor a BMC configured", server.Name)
}

func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1alpha1.BMC, insecure bool, options bmc.Options, opts ...BMCClientOptions) (bmc.BMC, error) {
func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1alpha1.BMC, defaultProtocol metalv1alpha1.ProtocolScheme, skipCertValidation bool, options bmc.Options, opts ...BMCClientOptions) (bmc.BMC, error) {
var address string

if !slices.Contains(opts, BMCConnectivityCheckOption) {
Expand All @@ -170,8 +168,8 @@ func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1al
return nil, fmt.Errorf("failed to get BMC secret: %w", err)
}

protocolScheme := GetProtocolScheme(bmcObj.Spec.Protocol.Scheme, insecure)
bmcClient, err := CreateBMCClient(ctx, c, protocolScheme, bmcObj.Spec.Protocol.Name, address, bmcObj.Spec.Protocol.Port, bmcSecret, options)
protocolScheme := GetProtocolScheme(bmcObj.Spec.Protocol.Scheme, defaultProtocol)
bmcClient, err := CreateBMCClient(ctx, c, protocolScheme, bmcObj.Spec.Protocol.Name, address, bmcObj.Spec.Protocol.Port, bmcSecret, options, skipCertValidation)
return bmcClient, err
}

Expand All @@ -184,6 +182,7 @@ func CreateBMCClient(
port int32,
bmcSecret *metalv1alpha1.BMCSecret,
bmcOptions bmc.Options,
skipCertValidation bool,
) (bmc.BMC, error) {
var bmcClient bmc.BMC
var err error
Expand All @@ -193,6 +192,7 @@ func CreateBMCClient(
if err != nil {
return nil, fmt.Errorf("failed to get credentials from BMC secret: %w", err)
}
bmcOptions.InsecureTLS = skipCertValidation

switch bmcProtocol {
case metalv1alpha1.ProtocolRedfish:
Expand Down
17 changes: 9 additions & 8 deletions internal/controller/biossettings_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ const (
// BIOSSettingsReconciler reconciles a BIOSSettings object
type BIOSSettingsReconciler struct {
client.Client
ManagerNamespace string
Insecure bool
Scheme *runtime.Scheme
BMCOptions bmc.Options
ResyncInterval time.Duration
TimeoutExpiry time.Duration
Conditions *conditionutils.Accessor
ManagerNamespace string
DefaultProtocol metalv1alpha1.ProtocolScheme
SkipCertValidation bool
Scheme *runtime.Scheme
BMCOptions bmc.Options
ResyncInterval time.Duration
TimeoutExpiry time.Duration
Conditions *conditionutils.Accessor
}

// +kubebuilder:rbac:groups=metal.ironcore.dev,resources=biossettings,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -269,7 +270,7 @@ func (r *BIOSSettingsReconciler) reconcile(ctx context.Context, settings *metalv
return ctrl.Result{}, err
}

bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions)
if err != nil {
if errors.As(err, &bmcutils.BMCUnAvailableError{}) {
log.V(1).Info("BMC is not available", "BMC", server.Spec.BMCRef.Name, "Server", server.Name, "Message", err.Error())
Expand Down
15 changes: 8 additions & 7 deletions internal/controller/biosversion_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ import (
// BIOSVersionReconciler reconciles a BIOSVersion object
type BIOSVersionReconciler struct {
client.Client
ManagerNamespace string
Insecure bool
Scheme *runtime.Scheme
BMCOptions bmc.Options
ResyncInterval time.Duration
Conditions *conditionutils.Accessor
ManagerNamespace string
DefaultProtocol metalv1alpha1.ProtocolScheme
SkipCertValidation bool
Scheme *runtime.Scheme
BMCOptions bmc.Options
ResyncInterval time.Duration
Conditions *conditionutils.Accessor
}

const (
Expand Down Expand Up @@ -182,7 +183,7 @@ func (r *BIOSVersionReconciler) transitionState(ctx context.Context, biosVersion
return false, fmt.Errorf("failed to fetch server: %w", err)
}

bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.Insecure, r.BMCOptions)
bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions)
if err != nil {
if errors.As(err, &bmcutils.BMCUnAvailableError{}) {
log.V(1).Info("BMC is not available, skipping", "BMC", server.Spec.BMCRef.Name, "Server", server.Name, "error", err)
Expand Down
9 changes: 5 additions & 4 deletions internal/controller/bmc_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ const (
// BMCReconciler reconciles a BMC object
type BMCReconciler struct {
client.Client
Scheme *runtime.Scheme
Insecure bool
Scheme *runtime.Scheme
DefaultProtocol metalv1alpha1.ProtocolScheme
SkipCertValidation bool
// BMCFailureResetDelay defines the duration after which a BMC will be reset upon repeated connection failures.
BMCFailureResetDelay time.Duration
BMCOptions bmc.Options
Expand Down Expand Up @@ -109,7 +110,7 @@ func (r *BMCReconciler) delete(ctx context.Context, bmcObj *metalv1alpha1.BMC) (
}
}

bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.Insecure, r.BMCOptions)
bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions)
if err == nil {
defer bmcClient.Logout()
if err := r.deleteEventSubscription(ctx, bmcClient, bmcObj); err != nil {
Expand Down Expand Up @@ -142,7 +143,7 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC
RequeueAfter: r.BMCClientRetryInterval,
}, nil
}
bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.Insecure, r.BMCOptions, bmcutils.BMCConnectivityCheckOption)
bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, bmcutils.BMCConnectivityCheckOption)
if err != nil {
if r.shouldResetBMC(bmcObj) {
log.V(1).Info("BMC needs reset, resetting", "BMC", bmcObj.Name)
Expand Down
Loading
Loading