Skip to content
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.

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

[v24.3.x] rpk: improve error messages in security/acl #24331

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions src/go/rpk/pkg/adminapi/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion src/go/rpk/pkg/cli/security/acl/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
})
}
},
Expand Down
2 changes: 1 addition & 1 deletion src/go/rpk/pkg/cli/security/role/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion tests/rptest/tests/acls_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down