Skip to content

AGENT-1407: add support for InternalReleaseImage related resources #8600

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
andfasano wants to merge 2 commits into
base: master
Choose a base branch
from
Open

AGENT-1407: add support for InternalReleaseImage related resources #8600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
821af24
vendoring github.com/openshift/api to v0.0.0-20251203133830-85216507eada
andfasano Dec 22, 2025
6e0a31a
add support for InternalReleaseImage related resources
andfasano Dec 18, 2025
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ require (
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.35.1
github.com/openshift-online/ocm-sdk-go v0.1.205
github.com/openshift/api v0.0.0-20251117165054-348370f055bf
github.com/openshift/api v0.0.0-20251203133830-85216507eada
github.com/openshift/assisted-image-service v0.0.0-20231023144959-c402402f52bf
github.com/openshift/assisted-service/api v0.0.0
github.com/openshift/assisted-service/client v0.0.0
Expand Down
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 @@ -1025,8 +1025,8 @@ github.com/openshift-online/ocm-sdk-go v0.1.205/go.mod h1:iOhkt/6nFp9v7hasdmGbYl
github.com/openshift/api v0.0.0-20200326160804-ecb9283fe820/go.mod h1:RKMJ5CBnljLfnej+BJ/xnOWc3kZDvJUaIAEq2oKSPtE=
github.com/openshift/api v0.0.0-20200827090112-c05698d102cf/go.mod h1:M3xexPhgM8DISzzRpuFUy+jfPjQPIcs9yqEYj17mXV8=
github.com/openshift/api v0.0.0-20200829102639-8a3a835f1acf/go.mod h1:M3xexPhgM8DISzzRpuFUy+jfPjQPIcs9yqEYj17mXV8=
github.com/openshift/api v0.0.0-20251117165054-348370f055bf h1:8VzLlQFneh4bnHA3SS+Bb9VWdVaR7WugtSeqIngMC3s=
github.com/openshift/api v0.0.0-20251117165054-348370f055bf/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
github.com/openshift/api v0.0.0-20251203133830-85216507eada h1:Bo2v12aq3pj+kMger/lPONlq7vBJpLz4B9cZnLdZdks=
github.com/openshift/api v0.0.0-20251203133830-85216507eada/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
github.com/openshift/assisted-image-service v0.0.0-20231023144959-c402402f52bf h1:cBRHiPeMKGWX1ml1rLvMm1ILTJQqLC0AVhTDSJIaIDY=
github.com/openshift/assisted-image-service v0.0.0-20231023144959-c402402f52bf/go.mod h1:C/h+zRheayk9F6NxH7t2D26p++o4bRpMf/CV4Lovp9E=
github.com/openshift/baremetal-operator/apis v0.0.0-20231019133159-8643f32fea3e h1:wxkrIEx1lA6vdGAuBAgXmOVDJ+vX+4nhc7Cgu2jszSg=
Expand Down
60 changes: 60 additions & 0 deletions internal/ignition/installmanifests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,22 @@ import (
"os/exec"
"path/filepath"
"sort"
"strconv"
"strings"

config_latest_types "github.com/coreos/ignition/v2/config/v3_2/types"
"github.com/go-openapi/strfmt"
"github.com/go-openapi/swag"
"github.com/google/uuid"
bmh_v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
v1 "github.com/openshift/api/config/v1"
"github.com/openshift/api/features"
"github.com/openshift/assisted-service/internal/common"
ignitioncommon "github.com/openshift/assisted-service/internal/common/ignition"
"github.com/openshift/assisted-service/internal/constants"
eventsapi "github.com/openshift/assisted-service/internal/events/api"
"github.com/openshift/assisted-service/internal/host/hostutil"
"github.com/openshift/assisted-service/internal/installcfg"
"github.com/openshift/assisted-service/internal/installercache"
"github.com/openshift/assisted-service/internal/manifests"
manifestsapi "github.com/openshift/assisted-service/internal/manifests/api"
Expand Down Expand Up @@ -139,6 +143,8 @@ type installerGenerator struct {
installerCache *installercache.Installers
nodeIpAllocations map[strfmt.UUID]*network.NodeIpAllocation
manifestApi manifestsapi.ManifestsAPI
iriPatcher internalReleaseImagePatcher
rawInstallConfig []byte
}

var fileNames = [...]string{
Expand Down Expand Up @@ -170,6 +176,7 @@ func NewGenerator(workDir string, cluster *common.Cluster, releaseImage string,
clusterTLSCertOverrideDir: clusterTLSCertOverrideDir,
installerCache: installerCache,
manifestApi: manifestApi,
iriPatcher: NewInternalReleaseImagePatcher(cluster, s3Client, manifestApi, log),
}
}

Expand All @@ -179,6 +186,18 @@ func (g *installerGenerator) UploadToS3(ctx context.Context) error {
return uploadToS3(ctx, g.workDir, g.cluster, g.s3Client, g.log)
}

func (g *installerGenerator) patchInternalReleaseManifests(ctx context.Context, manifestFiles []s3wrapper.ObjectInfo) error {
if !g.isFeatureGateEnabled(features.FeatureGateNoRegistryClusterInstall) {
return nil
}

if err := g.iriPatcher.PatchManifests(ctx, manifestFiles); err != nil {
g.log.WithError(err).Errorf("failed to process manifests for cluster %s", g.cluster.ID)
return err
}
return nil
}

func (g *installerGenerator) allocateNodeIpsIfNeeded(log logrus.FieldLogger) {
if common.IsMultiNodeNonePlatformCluster(g.cluster) || network.IsLoadBalancerUserManaged(g.cluster) {
nodeIpAllocations, err := network.GenerateNonePlatformAddressAllocation(g.cluster, log)
Expand All @@ -194,6 +213,7 @@ func (g *installerGenerator) allocateNodeIpsIfNeeded(log logrus.FieldLogger) {
func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte, forceInsecurePolicyJson bool) error {
var err error
log := logutil.FromContext(ctx, g.log)
g.rawInstallConfig = installConfig

defer func() {
if err != nil {
Expand Down Expand Up @@ -270,6 +290,10 @@ func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte,
log.WithError(err).Errorf("failed to check if cluster %s has manifests", g.cluster.ID)
return err
}
err = g.patchInternalReleaseManifests(ctx, manifestFiles)
if err != nil {
return err
}

err = g.providerRegistry.PreCreateManifestsHook(g.cluster, &envVars, g.workDir)

Expand Down Expand Up @@ -790,6 +814,13 @@ func (g *installerGenerator) updateBootstrap(ctx context.Context, bootstrapPath
setNMConfigration(config)
}

if g.isFeatureGateEnabled(features.FeatureGateNoRegistryClusterInstall) {
err = g.iriPatcher.UpdateBootstrap(config)
if err != nil {
return err
}
}

err = ignitioncommon.WriteIgnitionFile(bootstrapPath, config)
if err != nil {
log.Error(err)
Expand Down Expand Up @@ -1326,6 +1357,35 @@ func (g *installerGenerator) downloadManifest(ctx context.Context, manifest stri
return nil
}

func (g *installerGenerator) isFeatureGateEnabled(feature v1.FeatureGateName) bool {
var installConfig installcfg.InstallerConfigBaremetal

if err := json.Unmarshal(g.rawInstallConfig, &installConfig); err != nil {
g.log.Errorf("cannot convert install config data while checking feature gate %s", string(feature))
return false
}
for _, fg := range installConfig.FeatureGates {
// Parse feature gate
featureParts := strings.Split(fg, "=")
if len(featureParts) != 2 {
g.log.Debugf("Cannot parse feature gate %s, skipping", fg)
continue
}
featureName := featureParts[0]
featureEnabled, err := strconv.ParseBool(featureParts[1])
if err != nil {
g.log.Debugf("Unsupported feature value %s, skipping", fg)
continue
}

if featureName == string(feature) && featureEnabled {
g.log.Debugf("Feature gate %s is enabled", string(feature))
return true
}
}
return false
}
Comment on lines +1360 to +1387
Copy link

@coderabbitai coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent error handling could mask configuration issues.

The isFeatureGateEnabled method logs but doesn't return errors from json.Unmarshal (line 1363), causing it to return false even when the install config is malformed. This could mask configuration errors that should be surfaced.

Additionally, the manual parsing of feature gates (lines 1367-1379) via string splitting could be fragile. Consider whether there's a standard utility in the OpenShift API for parsing feature gates, or at least add more robust error handling for malformed feature gate strings.

🔎 Suggested improvements
  1. Consider returning errors from isFeatureGateEnabled:
-func (g *installerGenerator) isFeatureGateEnabled(feature v1.FeatureGateName) bool {
+func (g *installerGenerator) isFeatureGateEnabled(feature v1.FeatureGateName) (bool, error) {
 	var installConfig installcfg.InstallerConfigBaremetal

 	if err := json.Unmarshal(g.rawInstallConfig, &installConfig); err != nil {
 		g.log.Errorf("cannot convert install config data while checking feature gate %s", string(feature))
-		return false
+		return false, err
 	}

Then update callers to handle the error.

  1. Add more robust parsing:
 	for _, fg := range installConfig.FeatureGates {
 		// Parse feature gate
-		featureParts := strings.Split(fg, "=")
+		featureParts := strings.SplitN(fg, "=", 2)
 		if len(featureParts) != 2 {
 			g.log.Debugf("Cannot parse feature gate %s, skipping", fg)
 			continue
 		}
 		featureName := featureParts[0]
-		featureEnabled, err := strconv.ParseBool(featureParts[1])
+		// strconv.ParseBool accepts "1", "t", "T", "TRUE", "true", "True", etc.
+		featureEnabled, err := strconv.ParseBool(strings.TrimSpace(featureParts[1]))
 		if err != nil {
 			g.log.Debugf("Unsupported feature value %s, skipping", fg)
 			continue
 		}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In internal/ignition/installmanifests.go around lines 1360 to 1387, the
isFeatureGateEnabled function swallows json.Unmarshal errors and returns false
for malformed configs and uses fragile string-splitting for feature gate
parsing; change the signature to return (bool, error) so callers can handle JSON
unmarshal failures (propagate and surface the actual error), log or wrap the
unmarshalling error with context instead of silently returning false, and
replace the manual split with stricter parsing/validation (or call the OpenShift
feature-gate parsing utility if available) so malformed entries produce a clear
error/log and are skipped safely; update all callers to handle the new (bool,
error) return and adjust unit tests accordingly.


// UploadToS3 uploads the generated files to S3
func uploadToS3(ctx context.Context, workDir string, cluster *common.Cluster, s3Client s3wrapper.API, log logrus.FieldLogger) error {
toUpload := fileNames[:]
Expand Down
Loading