From 0fdeb53a0c366d868bb4205797dbc67005614adc Mon Sep 17 00:00:00 2001 From: r-vasquez Date: Mon, 25 Nov 2024 10:40:00 -0800 Subject: [PATCH 1/2] rpk: add err message of CreateACL response Response of CreateACL contains an ErrMessage field that can provide more information about unknown failures. (cherry picked from commit ffbd1d3969b149d34e4830e5d9df12255ef69bc5) --- src/go/rpk/pkg/cli/security/acl/create.go | 6 +++++- tests/rptest/tests/acls_test.py | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/go/rpk/pkg/cli/security/acl/create.go b/src/go/rpk/pkg/cli/security/acl/create.go index c23f38b301a32..4d556f69d0369 100644 --- a/src/go/rpk/pkg/cli/security/acl/create.go +++ b/src/go/rpk/pkg/cli/security/acl/create.go @@ -69,6 +69,10 @@ Allow write permissions to user buzz to transactional ID "txn": tw := out.NewTable(headersWithError...) defer tw.Flush() for _, c := range results { + errMsg := kafka.ErrMessage(c.Err) + if c.ErrMessage != "" { + errMsg = fmt.Sprintf("%v: %v", errMsg, c.ErrMessage) + } tw.PrintStructFields(aclWithMessage{ c.Principal, c.Host, @@ -77,7 +81,7 @@ Allow write permissions to user buzz to transactional ID "txn": c.Pattern, c.Operation, c.Permission, - kafka.ErrMessage(c.Err), + errMsg, }) } }, diff --git a/tests/rptest/tests/acls_test.py b/tests/rptest/tests/acls_test.py index f6629efe3c50c..1d829648648ae 100644 --- a/tests/rptest/tests/acls_test.py +++ b/tests/rptest/tests/acls_test.py @@ -307,7 +307,8 @@ def test_invalid_acl_topic_name(self): acl = [acl for acl in acls if acl.resource_name == resource] assert len(acl) == 1, f'Expected match for {resource} not found' - assert acl[0].error == 'INVALID_REQUEST' + assert 'INVALID_REQUEST' in acl[ + 0].error, f'expected INVALID_REQUEST to be in {acl[0].error}' ''' The old config style has use_sasl at the top level, which enables From 690cce38e0ddb7557b03fb1974d8925042cbdffe Mon Sep 17 00:00:00 2001 From: r-vasquez Date: Mon, 25 Nov 2024 12:42:11 -0800 Subject: [PATCH 2/2] rpk: decode adminapi error message in role creation (cherry picked from commit b6628faadc2956f451c59694c15c6f2cde41269f) --- src/go/rpk/pkg/adminapi/admin.go | 16 ++++++++++++++++ src/go/rpk/pkg/cli/security/role/create.go | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/go/rpk/pkg/adminapi/admin.go b/src/go/rpk/pkg/adminapi/admin.go index 8c67799a932ea..73d30c4850e2e 100644 --- a/src/go/rpk/pkg/adminapi/admin.go +++ b/src/go/rpk/pkg/adminapi/admin.go @@ -131,6 +131,22 @@ func NewHostClient(fs afero.Fs, p *config.RpkProfile, host string) (*rpadmin.Adm return rpadmin.NewClient(addrs, tc, auth, p.FromCloud) } +// TryDecodeMessageFromErr tries to decode the message if it's a +// rpadmin.HTTPResponseError and logs the full error. Otherwise, it returns +// the original error string. +func TryDecodeMessageFromErr(err error) string { + if err == nil { + return "" + } + if he := (*rpadmin.HTTPResponseError)(nil); errors.As(err, &he) { + zap.L().Sugar().Debugf("got admin API error: %v", strings.TrimSpace(err.Error())) + if body, err := he.DecodeGenericErrorBody(); err == nil { + return body.Message + } + } + return strings.TrimSpace(err.Error()) +} + // licenseFeatureChecks checks if the user is talking to a cluster that has // enterprise features enabled without a license and returns a message with a // warning. diff --git a/src/go/rpk/pkg/cli/security/role/create.go b/src/go/rpk/pkg/cli/security/role/create.go index 339c29a8e0013..1aa9c879c16e3 100644 --- a/src/go/rpk/pkg/cli/security/role/create.go +++ b/src/go/rpk/pkg/cli/security/role/create.go @@ -46,7 +46,7 @@ flag in the 'rpk security acl create' command.`, roleName := args[0] _, err = cl.CreateRole(cmd.Context(), roleName) - out.MaybeDie(err, "unable to create role %q: %v", roleName, err) + out.MaybeDie(err, "unable to create role %q: %v", roleName, adminapi.TryDecodeMessageFromErr(err)) if isText, _, s, err := f.Format(createResponse{[]string{roleName}}); !isText { out.MaybeDie(err, "unable to print in the required format %q: %v", f.Kind, err)