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.

Sign up for GitHub

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

Jump to bottom

Ignore not-supported result from RetrieveUserGroups in VC 6.0 #7328

Merged
merged 3 commits into from
Feb 20, 2018
+26 −4
Merged

Ignore not-supported result from RetrieveUserGroups in VC 6.0 #7328

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46b4198
Ignore not-supported result from RetrieveUserGroups in VC 6.0
lcastellano Feb 14, 2018
8014b89
Merge branch 'master' into ops-user
lcastellano Feb 16, 2018
69b4f21
Merge branch 'master' into ops-user
lcastellano Feb 20, 2018
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
19 changes: 17 additions & 2 deletions pkg/vsphere/rbac/rbac.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,12 @@ func (am *AuthzManager) PrincipalBelongsToGroup(ctx context.Context, group strin
}

results, err := methods.RetrieveUserGroups(ctx, am.client, &req)

// This is to work around a bug in vSphere, when AD is added to
// the identity source list, the API returns Object Not Found,
// In this case, we ignore the error and return false (BUG: 2037706)
if err != nil && isNotFoundError(ctx, err) {
op.Debugf("Received Not Found Error from PrincipalBelongsToGroup(), could not verify user %s is not a member of the Administrators group", am.Principal)
if err != nil && (isNotSupportedError(ctx, err) || isNotFoundError(ctx, err)) {
op.Debugf("Received Error (%s) from PrincipalBelongsToGroup(), could not verify user %s is not a member of the Administrators group", err.Error(), am.Principal)
op.Warnf("If ops-user (%s) belongs to the Administrators group, permissions on some resources might have been restricted", am.Principal)
return false, nil
}
Expand Down Expand Up @@ -393,6 +394,20 @@ func (am *AuthzManager) getRoleName(resource *Resource) string {
}
}

func isNotSupportedError(ctx context.Context, err error) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like this and isNotFound should roll into base govmomi somehow. @dougm?
Not needed for this PR unless already available in govmomi.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

govmomi probably should have some of these error helpers, but it doesn't other than types.IsFileNotFound that VIC already makes use of.

op := trace.FromContext(ctx, "isNotSupportedError")

if soap.IsSoapFault(err) {
vimFault := soap.ToSoapFault(err).VimFault()
op.Debugf("Error type: %s", reflect.TypeOf(vimFault))

_, ok := soap.ToSoapFault(err).VimFault().(types.NotSupported)
return ok
}

return false
}

func isNotFoundError(ctx context.Context, err error) bool {
op := trace.FromContext(ctx, "isNotFoundError")

Expand Down
7 changes: 7 additions & 0 deletions tests/manual-test-cases/Group5-Functional-Tests/5-25-OPS-User-Grant.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,15 @@ Documentation Test 5-25 - OPS-User-Grant
Resource ../../resources/Util.robot
Suite Setup Wait Until Keyword Succeeds 10x 10m Ops User Create
Suite Teardown Run Keyword And Ignore Error Nimbus Cleanup ${list}
Test Teardown Run Keyword If Test Failed Gather VC Logs

*** Keywords ***

Gather VC Logs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go into one of the utility libraries.
I'm also unsure whether it will download the ESX log bundles or just VC - a keyword comment noting which would be good.

Log To Console Collecting VC logs ..
Run Keyword And Ignore Error Gather Logs From ESX Server
Log To Console VC logs collected

Ops User Create
[Timeout] 110 minutes
Run Keyword And Ignore Error Nimbus Cleanup ${list} ${false}
Expand Down
4 changes: 2 additions & 2 deletions tests/nightly/upload-logs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ outfile="vic_nightly_logs_"$1".zip"
echo $outfile

if [ -d "60" ]; then
/usr/bin/zip -9 -r $outfile 60 *.zip *.log *.debug
/usr/bin/zip -9 -r $outfile 60 *.zip *.log *.debug *.tgz
elif [ -d "65" ]; then
/usr/bin/zip -9 -r $outfile 65 *.zip *.log *.debug
/usr/bin/zip -9 -r $outfile 65 *.zip *.log *.debug *.tgz
else
echo "No output directories to upload!"
exit 1
Expand Down