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
19 changes: 19 additions & 0 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5001,6 +5001,25 @@ spec:
- cidr
type: object
type: array
networkObservability:
description: |-
NetworkObservability is an optional field that configures network observability installation
during cluster deployment (day-0).
When omitted, network observability will be installed unless this is a SNO cluster.
properties:
installationPolicy:
description: |-
InstallationPolicy controls whether network observability is installed during cluster deployment.
Valid values are "", "InstallAndEnable" and "DoNotInstall".
When set to "", network observability will be installed unless this is a SNO cluster.
When set to "InstallAndEnable", network observability will be installed and enabled.
When set to "DoNotInstall", network observability will not be installed.
enum:
- ""
- InstallAndEnable
- DoNotInstall
type: string
type: object
Comment on lines +5009 to +5022
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle explicit networkObservability: {} deterministically.

installationPolicy is optional here, but installer defaulting only runs when the whole networkObservability object is nil (pkg/types/defaults/installconfig.go:59-66), and manifest emission only runs when installationPolicy is non-nil (pkg/asset/manifests/network.go:83-88). An explicit empty object can therefore bypass day-0 defaulting and leave behavior implicit downstream.

Please either require installationPolicy in this schema or ensure defaulting fills it when networkObservability is present but empty, and add a regression test for networkObservability: {}.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@data/data/install.openshift.io_installconfigs.yaml` around lines 5009 - 5022,
The schema allows an explicit empty networkObservability object to bypass
defaults and manifest emission; either make installationPolicy required in the
networkObservability schema (so installationPolicy must be present) or change
the defaulting logic so that when networkObservability is present but has no
installationPolicy (i.e., networkObservability: {}), the defaulting routine that
sets installationPolicy (see installconfig defaulting code) populates a value;
after implementing the schema or defaulting change, add a regression test that
validates behavior for networkObservability: {} to ensure installationPolicy is
set and manifests are emitted as expected.

networkType:
default: OVNKubernetes
description: |-
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -383,3 +383,5 @@ replace (
github.com/nutanix-cloud-native/cluster-api-provider-nutanix => github.com/nutanix-cloud-native/cluster-api-provider-nutanix v1.7.2-0.20251007022949-442bc2ebe286
sigs.k8s.io/cluster-api-provider-azure => github.com/mboersma/cluster-api-provider-azure v0.3.1-0.20251030205607-3161b9cc8d3e
)

replace github.com/openshift/api v0.0.0-20260318185450-1f2fa3f09f4e => github.com/OlivierCazade/api v0.0.0-20260325001208-9b6df7f19a28
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ github.com/Masterminds/sprig/v3 v3.3.0/go.mod h1:Zy1iXRYNqNLUolqCpL4uhk6SHUMAOSC
github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ=
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63nhn5WAunQHLTznkw5W8b1Xc0dNjp83s=
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w=
github.com/OlivierCazade/api v0.0.0-20260325001208-9b6df7f19a28 h1:G2zLQb0xZ4ZgrnHQvYi5B/hUXxfFXdikdYPPjw/nB/0=
github.com/OlivierCazade/api v0.0.0-20260325001208-9b6df7f19a28/go.mod h1:pyVjK0nZ4sRs4fuQVQ4rubsJdahI1PB94LnQ8sGdvxo=
github.com/OpenPeeDeeP/depguard/v2 v2.2.1 h1:vckeWVESWp6Qog7UZSARNqfu/cZqvki8zsuj3piCMx4=
github.com/OpenPeeDeeP/depguard/v2 v2.2.1/go.mod h1:q4DKzC4UcVaAvcfd41CZh0PWpGgzrVxUYBlgKNGquUo=
github.com/PaesslerAG/gval v1.0.0 h1:GEKnRwkWDdf9dOmKcNrar9EA1bz1z9DqPIO1+iLzhd8=
Expand Down Expand Up @@ -888,8 +890,6 @@ github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJw
github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M=
github.com/opencontainers/runtime-spec v1.2.0 h1:z97+pHb3uELt/yiAWD691HNHQIF07bE7dzrbT927iTk=
github.com/opencontainers/runtime-spec v1.2.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
github.com/openshift/api v0.0.0-20260318185450-1f2fa3f09f4e h1:n2fW82JRX5B/+eCULWIe06MAhJVaE8fUsvy0A0Gn9n4=
github.com/openshift/api v0.0.0-20260318185450-1f2fa3f09f4e/go.mod h1:pyVjK0nZ4sRs4fuQVQ4rubsJdahI1PB94LnQ8sGdvxo=
github.com/openshift/assisted-image-service v0.0.0-20250917153356-4ca9ff81f712 h1:UJVh+I/AWZcOJASGdiLcTXkWB1OYNhS/383DHMcRvCQ=
github.com/openshift/assisted-image-service v0.0.0-20250917153356-4ca9ff81f712/go.mod h1:WGdSeSnK0voEWWwA4ar5eApNjGBLmGTpFurEKw/FXJc=
github.com/openshift/assisted-service/api v0.0.0-20250922204150-a52b83145bea h1:YhJ9iHKKT5ooAdVr8qq3BdudhTxP/WF0XYDT5gzi1ak=
Expand Down
18 changes: 18 additions & 0 deletions pkg/asset/agent/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ import (
"github.com/openshift/installer/pkg/types/vsphere"
)

var (
defaultNetworkObservabilityInstallAndEnable = types.NetworkObservabilityInstallAndEnable
defaultNetworkObservability = &types.NetworkObservability{
InstallationPolicy: &defaultNetworkObservabilityInstallAndEnable,
}
)

func TestInstallConfigLoad(t *testing.T) {
cases := []struct {
name string
Expand Down Expand Up @@ -469,6 +476,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1058,6 +1066,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1145,6 +1154,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1212,6 +1222,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1279,6 +1290,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1346,6 +1358,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1451,6 +1464,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1596,6 +1610,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -1809,6 +1824,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -2056,6 +2072,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -2159,6 +2176,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"c3VwZXItc2VjcmV0Cg==\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down
11 changes: 11 additions & 0 deletions pkg/asset/installconfig/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ import (
"github.com/openshift/installer/pkg/types/none"
)

var (
defaultNetworkObservabilityInstallAndEnable = types.NetworkObservabilityInstallAndEnable
defaultNetworkObservability = &types.NetworkObservability{
InstallationPolicy: &defaultNetworkObservabilityInstallAndEnable,
}
)

func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
sshPublicKey := &sshPublicKey{}
baseDomain := &baseDomain{"test-domain", types.ExternalPublishingStrategy}
Expand Down Expand Up @@ -59,6 +66,7 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -126,6 +134,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -226,6 +235,7 @@ wrong_key: wrong_value
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down Expand Up @@ -281,6 +291,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
HostPrefix: 23,
},
},
NetworkObservability: defaultNetworkObservability,
},
ControlPlane: &types.MachinePool{
Name: "master",
Expand Down
27 changes: 18 additions & 9 deletions pkg/asset/manifests/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,23 @@ func (no *Networking) Generate(_ context.Context, dependencies asset.Parents) er
serviceNet = append(serviceNet, sn.String())
}

networkSpec := configv1.NetworkSpec{
ClusterNetwork: clusterNet,
ServiceNetwork: serviceNet,
NetworkType: netConfig.NetworkType,
// Block all Service.ExternalIPs by default
ExternalIP: &configv1.ExternalIPConfig{
Policy: &configv1.ExternalIPPolicy{},
},
}

// Set networkObservability from the install config
if netConfig.NetworkObservability != nil && netConfig.NetworkObservability.InstallationPolicy != nil {
networkSpec.NetworkObservability = configv1.NetworkObservabilitySpec{
InstallationPolicy: (*configv1.NetworkObservabilityInstallationPolicy)(netConfig.NetworkObservability.InstallationPolicy),
}
}

no.Config = &configv1.Network{
TypeMeta: metav1.TypeMeta{
APIVersion: configv1.SchemeGroupVersion.String(),
Expand All @@ -79,15 +96,7 @@ func (no *Networking) Generate(_ context.Context, dependencies asset.Parents) er
Name: "cluster",
// not namespaced
},
Spec: configv1.NetworkSpec{
ClusterNetwork: clusterNet,
ServiceNetwork: serviceNet,
NetworkType: netConfig.NetworkType,
// Block all Service.ExternalIPs by default
ExternalIP: &configv1.ExternalIPConfig{
Policy: &configv1.ExternalIPPolicy{},
},
},
Spec: networkSpec,
}

configData, err := yaml.Marshal(no.Config)
Expand Down
6 changes: 6 additions & 0 deletions pkg/types/defaults/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ func SetInstallConfigDefaults(c *types.InstallConfig) {
},
}
}
if c.Networking.NetworkObservability == nil {
installationPolicy := types.NetworkObservabilityInstallAndEnable
c.Networking.NetworkObservability = &types.NetworkObservability{
InstallationPolicy: &installationPolicy,
}
}
Comment on lines +61 to +66
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Default policy is not applied when NetworkObservability exists but InstallationPolicy is omitted

Line 61 only defaults when NetworkObservability is nil. If a user sets networkObservability: {}, InstallationPolicy remains nil, so the “default Enable” behavior is skipped.

Proposed fix
-	if c.Networking.NetworkObservability == nil {
-		installationPolicy := types.NetworkObservabilityInstallAndEnable
-		c.Networking.NetworkObservability = &types.NetworkObservability{
-			InstallationPolicy: &installationPolicy,
-		}
-	}
+	if c.Networking.NetworkObservability == nil {
+		c.Networking.NetworkObservability = &types.NetworkObservability{}
+	}
+	if c.Networking.NetworkObservability.InstallationPolicy == nil {
+		installationPolicy := types.NetworkObservabilityInstallAndEnable
+		c.Networking.NetworkObservability.InstallationPolicy = &installationPolicy
+	}
📝 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
if c.Networking.NetworkObservability == nil {
installationPolicy := types.NetworkObservabilityInstallAndEnable
c.Networking.NetworkObservability = &types.NetworkObservability{
InstallationPolicy: &installationPolicy,
}
}
if c.Networking.NetworkObservability == nil {
c.Networking.NetworkObservability = &types.NetworkObservability{}
}
if c.Networking.NetworkObservability.InstallationPolicy == nil {
installationPolicy := types.NetworkObservabilityInstallAndEnable
c.Networking.NetworkObservability.InstallationPolicy = &installationPolicy
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/types/defaults/installconfig.go` around lines 61 - 66, The current
defaulting only sets c.Networking.NetworkObservability when it's nil, so if
c.Networking.NetworkObservability exists but its InstallationPolicy is nil the
default is not applied; update the logic around
c.Networking.NetworkObservability to ensure that after ensuring the struct
exists (or creating it) you also check if
c.Networking.NetworkObservability.InstallationPolicy == nil and, if so, assign a
new variable set to types.NetworkObservabilityInstallAndEnable and assign its
address to InstallationPolicy; reference the existing symbols
c.Networking.NetworkObservability, InstallationPolicy, and
types.NetworkObservabilityInstallAndEnable when making this change.


if c.Publish == "" {
c.Publish = types.ExternalPublishingStrategy
Expand Down
37 changes: 37 additions & 0 deletions pkg/types/defaults/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
)

func defaultInstallConfig() *types.InstallConfig {
installationPolicy := types.NetworkObservabilityInstallAndEnable
return &types.InstallConfig{
AdditionalTrustBundlePolicy: defaultAdditionalTrustBundlePolicy(),
Networking: &types.Networking{
Expand All @@ -34,6 +35,9 @@ func defaultInstallConfig() *types.InstallConfig {
HostPrefix: int32(defaultHostPrefix),
},
},
NetworkObservability: &types.NetworkObservability{
InstallationPolicy: &installationPolicy,
},
},
ControlPlane: defaultMachinePool("master"),
Compute: []types.MachinePool{*defaultMachinePool("worker")},
Expand Down Expand Up @@ -285,6 +289,39 @@ func TestSetInstallConfigDefaults(t *testing.T) {
return c
}(),
},
{
name: "NetworkObservability nil",
config: &types.InstallConfig{
Networking: &types.Networking{
NetworkObservability: nil,
},
},
expected: func() *types.InstallConfig {
c := defaultInstallConfig()
return c
}(),
},
{
name: "NetworkObservability DoNotInstall",
config: &types.InstallConfig{
Networking: &types.Networking{
NetworkObservability: &types.NetworkObservability{
InstallationPolicy: func() *types.NetworkObservabilityInstallationPolicy {
p := types.NetworkObservabilityDoNotInstall
return &p
}(),
},
},
},
expected: func() *types.InstallConfig {
c := defaultInstallConfig()
doNotInstall := types.NetworkObservabilityDoNotInstall
c.Networking.NetworkObservability = &types.NetworkObservability{
InstallationPolicy: &doNotInstall,
}
return c
}(),
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
34 changes: 34 additions & 0 deletions pkg/types/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,13 @@ type Networking struct {
// pod network when NetworkType is set to OVNKubernetes.
OVNKubernetesConfig *OVNKubernetesConfig `json:"ovnKubernetesConfig,omitempty"`

// NetworkObservability is an optional field that configures network observability installation
// during cluster deployment (day-0).
// When omitted, network observability will be installed unless this is a SNO cluster.
//
// +optional
NetworkObservability *NetworkObservability `json:"networkObservability,omitempty"`

// Deprecated types, scheduled to be removed

// Deprecated way to configure an IP address pool for machines.
Expand Down Expand Up @@ -681,3 +688,30 @@ var OSImageStreamValues = []OSImageStream{
OSImageStreamRHCOS9,
OSImageStreamRHCOS10,
}

// NetworkObservabilityInstallationPolicy is an enumeration of the available network observability installation policies
// Valid values are "", "InstallAndEnable", "DoNotInstall".
// +kubebuilder:validation:Enum="";InstallAndEnable;DoNotInstall
type NetworkObservabilityInstallationPolicy string

const (
// NetworkObservabilityNoOpinion means that the user has no opinion and the platform is left
// to choose reasonable defaults. The current default is to install and enable network observability.
// This is subject to change over time.
NetworkObservabilityNoOpinion NetworkObservabilityInstallationPolicy = ""
// NetworkObservabilityInstallAndEnable means that network observability should be installed and enabled during cluster deployment
NetworkObservabilityInstallAndEnable NetworkObservabilityInstallationPolicy = "InstallAndEnable"
// NetworkObservabilityDoNotInstall means that network observability should not be installed
NetworkObservabilityDoNotInstall NetworkObservabilityInstallationPolicy = "DoNotInstall"
)

// NetworkObservability defines the configuration for network observability installation
type NetworkObservability struct {
// InstallationPolicy controls whether network observability is installed during cluster deployment.
// Valid values are "", "InstallAndEnable" and "DoNotInstall".
// When set to "", network observability will be installed unless this is a SNO cluster.
// When set to "InstallAndEnable", network observability will be installed and enabled.
// When set to "DoNotInstall", network observability will not be installed.
// +optional
InstallationPolicy *NetworkObservabilityInstallationPolicy `json:"installationPolicy,omitempty"`
}
26 changes: 26 additions & 0 deletions pkg/types/zz_generated.deepcopy.go

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

Loading