Skip to content

CORS-4308: Enable Network Observability during installation #10382

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

Open
OlivierCazade wants to merge 4 commits into
base: main
Choose a base branch
from
+96 −19
Open

CORS-4308: Enable Network Observability during installation #10382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6969ce2
Add observabilityEnabled field and test cases. Defaults to true.
stleerh Oct 7, 2025
9ed525e
Renamed field to InstallNetworkObservability
OlivierCazade Mar 10, 2026
643b775
Changing to the new type for InstallNetworkObservability
OlivierCazade Mar 11, 2026
79ef0ab
Pointing dependency to temporary branch to have the new flag before m…
OlivierCazade Mar 11, 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
10 changes: 10 additions & 0 deletions data/data/install.openshift.io_installconfigs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4791,6 +4791,16 @@ spec:
- cidr
type: object
type: array
installNetworkObservability:
description: |-
InstallNetworkObservability is an optional field that enables network observability
when omitted or set to "Enable". If the field is set to "Disable", it does nothing.
Valid values are "", "Enable", "Disable".
enum:
- ""
- Enable
- Disable
Comment on lines +4799 to +4802
Copy link

@coderabbitai coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid an undocumented empty-string mode.

The enum accepts "", but the description only defines behavior for omitted, "Enable", and "Disable". Please either document "" as equivalent to omitted/"Enable" or remove it from the allowed values so users do not get an ambiguous configuration state.

🤖 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 4799 - 4802,
The enum currently allows an empty-string value ("") alongside "Enable" and
"Disable" but the schema description only documents omitted, "Enable" and
"Disable"; update the enum to remove the blank entry or explicitly document the
blank as equivalent to the intended behavior (e.g., treat "" as omitted or as
"Enable") so there's no ambiguity—modify the enum list (remove the line with -
"") or add a sentence to the field description clarifying how "" is interpreted.

type: string
Comment on lines +4794 to +4803
Copy link

@coderabbitai coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Declare the default in the schema.

The PR defaults this field to "Enable" when it is omitted, but the CRD schema does not advertise that with a default: entry. That leaves schema-driven tooling and CRD defaulting out of sync with the documented behavior.

Suggested fix 
               installNetworkObservability:
                 description: |-
                   InstallNetworkObservability is an optional field that enables network observability
                   when omitted or set to "Enable". If the field is set to "Disable", it does nothing.
                   Valid values are "", "Enable", "Disable".
+                default: Enable
                 enum:
                 - ""
                 - Enable
                 - Disable
                 type: string
📝 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
installNetworkObservability:
description: |-
InstallNetworkObservability is an optional field that enables network observability
when omitted or set to "Enable". If the field is set to "Disable", it does nothing.
Valid values are "", "Enable", "Disable".
enum:
- ""
- Enable
- Disable
type: string
installNetworkObservability:
description: |-
InstallNetworkObservability is an optional field that enables network observability
when omitted or set to "Enable". If the field is set to "Disable", it does nothing.
Valid values are "", "Enable", "Disable".
default: Enable
enum:
- ""
- Enable
- Disable
type: string
🤖 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 4794 - 4803,
Add an explicit default for the installNetworkObservability property in the CRD
schema so schema-driven tooling matches runtime behavior: update the
installNetworkObservability property (the block with description, enum, and
type) to include a default: "Enable" entry (ensuring the string value matches
the enum entries), e.g., add default: Enable under the same indentation as
enum/type; keep the existing enum and type unchanged.

machineCIDR:
description: |-
Deprecated way to configure an IP address pool for machines.
Expand Down
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,3 +389,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-20260228183123-9b2ee997d297 => github.com/OlivierCazade/api v0.0.0-20260310172058-df85794acda8
Copy link

@coderabbitai coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== replace directive =="
sed -n '390,394p' go.mod

echo
echo "== repo imports that still resolve through github.com/openshift/api =="
rg -n -C1 '"github\.com/openshift/api/' --type=go -g '!vendor/**'

Repository: openshift/installer

Length of output: 50375

Do not merge with the personal-fork openshift/api replace still in place.

This makes the branch depend on github.com/OlivierCazade/api instead of an official github.com/openshift/api revision. The codebase imports from github.com/openshift/api across numerous packages (config/v1, machine/v1, machine/v1beta1, machineconfiguration/v1, features, operator/v1, and more), all of which would be satisfied by the temporary fork if merged. This introduces a supply-chain and reproducibility risk to main and must be reverted before merge.

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

In `@go.mod` at line 393, Remove the temporary replace directive that points
openshift/api to the personal fork: delete the line "replace
github.com/openshift/api v0.0.0-20260228183123-9b2ee997d297 =>
github.com/OlivierCazade/api v0.0.0-20260310172058-df85794acda8" from go.mod
and, if needed, run `go get` or `go mod tidy` to restore a proper official
github.com/openshift/api revision so all imports of github.com/openshift/api
(e.g., config/v1, machine/v1, machine/v1beta1, machineconfiguration/v1,
operator/v1) resolve to the upstream module rather than the fork.

4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
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-20260310172058-df85794acda8 h1:xum6axaZ9eg1yBb8IN1pVC93wL9puTiirC+KtH4Jw3E=
github.com/OlivierCazade/api v0.0.0-20260310172058-df85794acda8/go.mod h1:ZYAxo9t1AALeEotN07tNzIvqqqWSxcZIqMUKnY/xCeQ=
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 @@ -896,8 +898,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-20260228183123-9b2ee997d297 h1:QoHTB3QS859LUGE6NUTg98XiMz6Kzm3svQmo4tmgmlg=
github.com/openshift/api v0.0.0-20260228183123-9b2ee997d297/go.mod h1:ZYAxo9t1AALeEotN07tNzIvqqqWSxcZIqMUKnY/xCeQ=
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
25 changes: 16 additions & 9 deletions pkg/asset/manifests/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,21 @@ 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 installNetworkObservability from the install config
if netConfig.InstallNetworkObservability != nil {
networkSpec.InstallNetworkObservability = netConfig.InstallNetworkObservability
}

no.Config = &configv1.Network{
TypeMeta: metav1.TypeMeta{
APIVersion: configv1.SchemeGroupVersion.String(),
Expand All @@ -79,15 +94,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
4 changes: 4 additions & 0 deletions pkg/types/defaults/installconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ func SetInstallConfigDefaults(c *types.InstallConfig) {
},
}
}
if c.Networking.InstallNetworkObservability == nil {
installNetworkObservability := "Enable"
c.Networking.InstallNetworkObservability = &installNetworkObservability
}

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

func defaultInstallConfig() *types.InstallConfig {
installNetworkObservability := "Enable"
return &types.InstallConfig{
AdditionalTrustBundlePolicy: defaultAdditionalTrustBundlePolicy(),
Networking: &types.Networking{
Expand All @@ -34,6 +35,7 @@ func defaultInstallConfig() *types.InstallConfig {
HostPrefix: int32(defaultHostPrefix),
},
},
InstallNetworkObservability: &installNetworkObservability,
},
ControlPlane: defaultMachinePool("master"),
Compute: []types.MachinePool{*defaultMachinePool("worker")},
Expand Down Expand Up @@ -285,6 +287,32 @@ func TestSetInstallConfigDefaults(t *testing.T) {
return c
}(),
},
{
name: "InstallNetworkObservability nil",
config: &types.InstallConfig{
Networking: &types.Networking{
InstallNetworkObservability: nil,
},
},
expected: func() *types.InstallConfig {
c := defaultInstallConfig()
return c
}(),
},
{
name: "InstallNetworkObservability Disable",
config: &types.InstallConfig{
Networking: &types.Networking{
InstallNetworkObservability: func() *string { s := "Disable"; return &s }(),
},
},
expected: func() *types.InstallConfig {
c := defaultInstallConfig()
installNetworkObservability := "Disable"
c.Networking.InstallNetworkObservability = &installNetworkObservability
return c
}(),
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/types/installconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,14 @@ type Networking struct {
// pod network when NetworkType is set to OVNKubernetes.
OVNKubernetesConfig *OVNKubernetesConfig `json:"ovnKubernetesConfig,omitempty"`

// InstallNetworkObservability is an optional field that enables network observability
// when omitted or set to "Enable". If the field is set to "Disable", it does nothing.
// Valid values are "", "Enable", "Disable".
//
// +kubebuilder:validation:Enum="";Enable;Disable
// +optional
InstallNetworkObservability *string `json:"installNetworkObservability,omitempty"`
Comment on lines +429 to +435
Copy link

@coderabbitai coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== validate install-config entry points =="
fd 'installconfig\.go$' pkg/types/validation -x sed -n '120,220p' {}

echo
echo "== references to the new field in validation code =="
rg -n -C3 'InstallNetworkObservability|validateNetworking\s*\(' pkg/types/validation

Repository: openshift/installer

Length of output: 6902

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get the validateNetworking function starting from line 505
echo "=== validateNetworking function (full implementation) ==="
sed -n '505,700p' pkg/types/validation/installconfig.go

# Also check if InstallNetworkObservability is mentioned anywhere in the validation files
echo ""
echo "=== Search for InstallNetworkObservability in validation ==="
rg -n 'InstallNetworkObservability' pkg/types/validation/

Repository: openshift/installer

Length of output: 9479

Add enum validation for InstallNetworkObservability in the validation logic.

The validateNetworking() function in pkg/types/validation/installconfig.go does not validate the InstallNetworkObservability field. The kubebuilder tag documents the enum but does not enforce it at the installer level. Add an explicit check to reject values outside "", "Enable", and "Disable" to prevent invalid values from reaching manifest generation.

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

In `@pkg/types/installconfig.go` around lines 429 - 435, In validateNetworking(),
add an explicit validation for the InstallNetworkObservability field to reject
any value other than the allowed set ("", "Enable", "Disable"); locate the
InstallNetworkObservability pointer on the InstallConfig type and if non-nil
check its value and append a field error (using the same error accumulation
approach used in validateNetworking) for the json field
"installNetworkObservability" when it is not one of the three allowed strings so
invalid values are rejected before manifest generation.


// Deprecated types, scheduled to be removed

// Deprecated way to configure an IP address pool for machines.
Expand Down
5 changes: 5 additions & 0 deletions pkg/types/zz_generated.deepcopy.go
View file
Open in desktop

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

7 changes: 7 additions & 0 deletions vendor/github.com/openshift/api/config/v1/types_network.go
View file
Open in desktop

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

5 changes: 5 additions & 0 deletions vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go
View file
Open in desktop

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

15 changes: 8 additions & 7 deletions vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,7 @@ github.com/opencontainers/image-spec/specs-go/v1
# github.com/opencontainers/runtime-spec v1.2.0
## explicit
github.com/opencontainers/runtime-spec/specs-go
# github.com/openshift/api v0.0.0-20260228183123-9b2ee997d297
# github.com/openshift/api v0.0.0-20260228183123-9b2ee997d297 => github.com/OlivierCazade/api v0.0.0-20260310172058-df85794acda8
## explicit; go 1.24.0
github.com/openshift/api/annotations
github.com/openshift/api/config/v1
Expand Down