Skip to content

Commit 435db5e

Browse files
Merge pull request #10287 from zaneb/install-config-unmarshal-strict
CORS-3954: Enable strict unmarshaling for user invocations
2 parents 8c9ccde + bcf2811 commit 435db5e

3 files changed

Lines changed: 64 additions & 10 deletions

File tree

pkg/asset/imagebased/configimage/installconfig.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,21 @@ func (i *InstallConfig) loadFromFile(f asset.FileFetcher) (found bool, err error
6767
return false, fmt.Errorf("%s, err: %w", asset.InstallConfigError, err)
6868
}
6969
err = fmt.Errorf("failed to parse first occurrence of unknown field, err: %w", err)
70-
logrus.Warn(err.Error())
71-
logrus.Info("Attempting to unmarshal while ignoring unknown keys because strict unmarshaling failed")
72-
if err = yaml.Unmarshal(file.Data, config); err != nil {
73-
err = fmt.Errorf("failed to unmarshal %s, err: %w", InstallConfigFilename, err)
70+
71+
// Check if invoked by automation tool (Hive, assisted-service, etc.)
72+
// These tools may generate install-config with fields from newer versions
73+
// that aren't recognized by older installer versions.
74+
invoker := os.Getenv("OPENSHIFT_INSTALL_INVOKER")
75+
if invoker != "" {
76+
// Legacy behavior for automation tools: warn and continue
77+
logrus.Warn(err.Error())
78+
logrus.Info("Attempting to unmarshal while ignoring unknown keys because strict unmarshaling failed")
79+
if err = yaml.Unmarshal(file.Data, config); err != nil {
80+
err = fmt.Errorf("failed to unmarshal %s, err: %w", InstallConfigFilename, err)
81+
return false, fmt.Errorf("%s, err: %w", asset.InstallConfigError, err)
82+
}
83+
} else {
84+
// Strict behavior for direct user invocations: fail on unknown fields
7485
return false, fmt.Errorf("%s, err: %w", asset.InstallConfigError, err)
7586
}
7687
}

pkg/asset/installconfig/installconfig_test.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ func TestInstallConfigLoad(t *testing.T) {
8888
name string
8989
data string
9090
fetchError error
91+
invoker string
9192
expectedFound bool
9293
expectedError bool
9394
expectedConfig *types.InstallConfig
@@ -175,7 +176,7 @@ metadata:
175176
expectedError: true,
176177
},
177178
{
178-
name: "unknown field",
179+
name: "unknown field fails for user",
179180
data: `
180181
apiVersion: v1
181182
metadata:
@@ -185,7 +186,23 @@ baseDomain: test-domain
185186
platform:
186187
none: {}
187188
pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
188-
wrong_key: wrong_value
189+
wrong_key: wrong_value
190+
`,
191+
expectedError: true,
192+
},
193+
{
194+
name: "unknown field warns for automation tools",
195+
invoker: "hive",
196+
data: `
197+
apiVersion: v1
198+
metadata:
199+
name: test-cluster
200+
additionalTrustBundlePolicy: Proxyonly
201+
baseDomain: test-domain
202+
platform:
203+
none: {}
204+
pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
205+
wrong_key: wrong_value
189206
`,
190207
expectedFound: true,
191208
expectedConfig: &types.InstallConfig{
@@ -289,6 +306,21 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
289306
}
290307
for _, tc := range cases {
291308
t.Run(tc.name, func(t *testing.T) {
309+
// Set OPENSHIFT_INSTALL_INVOKER if specified, and restore it after the test
310+
oldInvoker, hadInvoker := os.LookupEnv("OPENSHIFT_INSTALL_INVOKER")
311+
if tc.invoker != "" {
312+
os.Setenv("OPENSHIFT_INSTALL_INVOKER", tc.invoker)
313+
} else {
314+
os.Unsetenv("OPENSHIFT_INSTALL_INVOKER")
315+
}
316+
defer func() {
317+
if hadInvoker {
318+
os.Setenv("OPENSHIFT_INSTALL_INVOKER", oldInvoker)
319+
} else {
320+
os.Unsetenv("OPENSHIFT_INSTALL_INVOKER")
321+
}
322+
}()
323+
292324
mockCtrl := gomock.NewController(t)
293325
defer mockCtrl.Finish()
294326

pkg/asset/installconfig/installconfigbase.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,21 @@ func (a *AssetBase) LoadFromFile(f asset.FileFetcher) (found bool, err error) {
5151
return false, errors.Wrap(err, asset.InstallConfigError)
5252
}
5353
err = errors.Wrapf(err, "failed to parse first occurrence of unknown field")
54-
logrus.Warnf("%s", err.Error())
55-
logrus.Info("Attempting to unmarshal while ignoring unknown keys because strict unmarshaling failed")
56-
if err = yaml.Unmarshal(file.Data, config); err != nil {
57-
err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename)
54+
55+
// Check if invoked by automation tool (Hive, assisted-service, etc.)
56+
// These tools may generate install-config with fields from newer versions
57+
// that aren't recognized by older installer versions.
58+
invoker := os.Getenv("OPENSHIFT_INSTALL_INVOKER")
59+
if invoker != "" {
60+
// Legacy behavior for automation tools: warn and continue
61+
logrus.Warnf("%s", err.Error())
62+
logrus.Info("Attempting to unmarshal while ignoring unknown keys because strict unmarshaling failed")
63+
if err = yaml.Unmarshal(file.Data, config); err != nil {
64+
err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename)
65+
return false, errors.Wrap(err, asset.InstallConfigError)
66+
}
67+
} else {
68+
// Strict behavior for direct user invocations: fail on unknown fields
5869
return false, errors.Wrap(err, asset.InstallConfigError)
5970
}
6071
}

0 commit comments

Comments
 (0)