Automated cherry pick of #130503: Unhandled panic crash on rollout_history printer.PrintObj (#131496)

* Change: Handling nil runtime.Object

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Return only if there is error in rollout_history

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Return the unknown revision error directly in rollout_history.go

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Remove unintended newline

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Using go idiomatic way for checking if historyInfo[o.Revision] exists

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Remove 'error:' from returned error message in rollout_history.go

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Check for printer.PrintObj returned err

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Add TestRolloutHistoryErrors test

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Simple typo fix on Complete() function description

Signed-off-by: Taha Farahani <tahacodes@proton.me>

* Change: Checking for error on o.Complete in TestRolloutHistoryErrors

Signed-off-by: Taha Farahani <tahacodes@proton.me>

---------

Signed-off-by: Taha Farahani <tahacodes@proton.me>
This commit is contained in:
Taha Farahani 2025-05-14 10:56:54 +03:30 committed by GitHub
parent 00ebe85a29
commit cce99a8c73
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 92 additions and 2 deletions

View File

@ -105,7 +105,7 @@ func NewCmdRolloutHistory(f cmdutil.Factory, streams genericiooptions.IOStreams)
return cmd return cmd
} }
// Complete completes al the required options // Complete completes all the required options
func (o *RolloutHistoryOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { func (o *RolloutHistoryOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
o.Resources = args o.Resources = args
@ -177,7 +177,15 @@ func (o *RolloutHistoryOptions) Run() error {
} }
if o.Revision > 0 { if o.Revision > 0 {
printer.PrintObj(historyInfo[o.Revision], o.Out) // Ensure the specified revision exists before printing
revision, exists := historyInfo[o.Revision]
if !exists {
return fmt.Errorf("unable to find the specified revision")
}
if err := printer.PrintObj(revision, o.Out); err != nil {
return err
}
} else { } else {
sortedKeys := make([]int64, 0, len(historyInfo)) sortedKeys := make([]int64, 0, len(historyInfo))
for k := range historyInfo { for k := range historyInfo {

View File

@ -401,6 +401,88 @@ replicaset.apps/rev2
} }
} }
func TestRolloutHistoryErrors(t *testing.T) {
ns := scheme.Codecs.WithoutConversion()
tf := cmdtesting.NewTestFactory().WithNamespace("test")
defer tf.Cleanup()
info, _ := runtime.SerializerInfoForMediaType(ns.SupportedMediaTypes(), runtime.ContentTypeJSON)
encoder := ns.EncoderForVersion(info.Serializer, rolloutPauseGroupVersionEncoder)
tf.Client = &RolloutPauseRESTClient{
RESTClient: &fake.RESTClient{
GroupVersion: rolloutPauseGroupVersionEncoder,
NegotiatedSerializer: ns,
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
switch p, m := req.URL.Path, req.Method; {
case p == "/namespaces/test/deployments/foo" && m == "GET":
responseDeployment := &appsv1.Deployment{}
responseDeployment.Name = "foo"
body := io.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(encoder, responseDeployment))))
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
default:
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
return nil, nil
}
}),
},
}
testCases := map[string]struct {
revision int64
outputFormat string
expectedError string
}{
"get non-existing revision as yaml": {
revision: 999,
outputFormat: "yaml",
expectedError: "unable to find the specified revision",
},
"get non-existing revision as json": {
revision: 999,
outputFormat: "json",
expectedError: "unable to find the specified revision",
},
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
fhv := setupFakeHistoryViewer(t)
fhv.getHistoryFn = func(namespace, name string) (map[int64]runtime.Object, error) {
return map[int64]runtime.Object{
1: &appsv1.ReplicaSet{ObjectMeta: v1.ObjectMeta{Name: "rev1"}},
2: &appsv1.ReplicaSet{ObjectMeta: v1.ObjectMeta{Name: "rev2"}},
}, nil
}
streams := genericiooptions.NewTestIOStreamsDiscard()
o := NewRolloutHistoryOptions(streams)
printFlags := &genericclioptions.PrintFlags{
JSONYamlPrintFlags: &genericclioptions.JSONYamlPrintFlags{
ShowManagedFields: true,
},
OutputFormat: &tc.outputFormat,
OutputFlagSpecified: func() bool {
return true
},
}
o.PrintFlags = printFlags
o.Revision = tc.revision
if err := o.Complete(tf, nil, []string{"deployment/foo"}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
err := o.Run()
if err != nil && err.Error() != tc.expectedError {
t.Fatalf("expected '%s' error, but got: %v", tc.expectedError, err)
}
})
}
}
func TestValidate(t *testing.T) { func TestValidate(t *testing.T) {
opts := RolloutHistoryOptions{ opts := RolloutHistoryOptions{
Revision: 0, Revision: 0,