-
Notifications
You must be signed in to change notification settings - Fork 282
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
libs/go/zmscli: fix dropped errors #2301
Conversation
libs/go/zmscli/import.go
Outdated
@@ -166,6 +166,9 @@ func (cli Zms) importRoles(dn string, lstRoles []*zms.Role, existingRoles *zms.R | |||
} | |||
} | |||
_, err = cli.AddRoleMembers(dn, rn, roleMembers) | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check here is not correct. we already handle the error on line 185 where we check if we should report errors or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error is unhandled, as a new err
is created inside the if
block on 152 when :=
is used.
athenz/libs/go/zmscli/import.go
Line 152 in 47a7734
adminRole, err := cli.Zms.GetRole(zms.DomainName(dn), "admin", nil, nil, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct fix here would be to define adminRole as a local variable and replace := with just = so we don't create a new err variable and mask the one from outside the if block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change made.
libs/go/zmscli/import.go
Outdated
@@ -234,6 +237,9 @@ func (cli Zms) importRolesOld(dn string, lstRoles []interface{}, validatedAdmins | |||
} | |||
} | |||
_, err = cli.AddRoleMembers(dn, rn, roleMembers) | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check here is not correct. we already handle the error on line 253 where we check if we should report errors or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This err
is declared inside the if
block when :=
is used, and is lost before the check on 253.
athenz/libs/go/zmscli/import.go
Line 222 in 47a7734
role, err := cli.Zms.GetRole(zms.DomainName(dn), "admin", nil, nil, nil) |
Signed-off-by: Lars Lehtonen <lars.lehtonen@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This fixes three dropped
err
variables in thelibs/go/zmscli
package.