Skip to content

RANGER-5551: TestRangerOzoneAuthorizer should use builder for RequestContext#913

Open
spacemonkd wants to merge 3 commits intoapache:masterfrom
spacemonkd:RANGER-5551
Open

RANGER-5551: TestRangerOzoneAuthorizer should use builder for RequestContext#913
spacemonkd wants to merge 3 commits intoapache:masterfrom
spacemonkd:RANGER-5551

Conversation

@spacemonkd
Copy link
Copy Markdown

What changes were proposed in this pull request?

Currently TestRangerOzoneAuthorizer passes the arguments directly to the Ozone RequestContext construcutor.
Example here

However on Ozone as part of PR #9493 we have updated the constructor to accept a Builder instance.
These tests need to be updated to pass this builder instead of direct arguments

How was this patch tested?

Patch was tested by running this test

Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @spacemonkd for the patch.

Comment on lines +111 to +150
RequestContext ctxListWithoutSessionPolicy = RequestContext.newBuilder()
.setHost(hostname)
.setIp(ipAddress)
.setClientUgi(user1)
.setServiceId(OZONE_SERVICE_ID)
.setAclType(IAccessAuthorizer.ACLIdentityType.ANONYMOUS)
.setAclRights(IAccessAuthorizer.ACLType.LIST)
.setOwnerName(OWNER_NAME)
.build();
RequestContext ctxReadWithoutSessionPolicy = RequestContext.newBuilder()
.setHost(hostname)
.setIp(ipAddress)
.setClientUgi(user1)
.setServiceId(OZONE_SERVICE_ID)
.setAclType(IAccessAuthorizer.ACLIdentityType.ANONYMOUS)
.setAclRights(IAccessAuthorizer.ACLType.READ)
.setOwnerName(OWNER_NAME)
.build();
RequestContext ctxListWithSessionPolicy = RequestContext.newBuilder()
.setHost(hostname)
.setIp(ipAddress)
.setClientUgi(user1)
.setServiceId(OZONE_SERVICE_ID)
.setAclType(IAccessAuthorizer.ACLIdentityType.ANONYMOUS)
.setAclRights(IAccessAuthorizer.ACLType.LIST)
.setOwnerName(OWNER_NAME)
.setRecursiveAccessCheck(false)
.setSessionPolicy(sessionPolicy)
.build();
RequestContext ctxReadWithSessionPolicy = RequestContext.newBuilder()
.setHost(hostname)
.setIp(ipAddress)
.setClientUgi(user1)
.setServiceId(OZONE_SERVICE_ID)
.setAclType(IAccessAuthorizer.ACLIdentityType.ANONYMOUS)
.setAclRights(IAccessAuthorizer.ACLType.READ)
.setOwnerName(OWNER_NAME)
.setRecursiveAccessCheck(false)
.setSessionPolicy(sessionPolicy)
.build();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Indentation looks very odd.
  • To reduce code duplication, we can create a base builder with the common properties, and build different RequestContext instances with tweaks.

Something like:

RequestContext.Builder builder = RequestContext.newBuilder()
    .setHost(hostname)
    .setIp(ipAddress)
    .setClientUgi(user1)
    .setServiceId(OZONE_SERVICE_ID)
    .setAclType(IAccessAuthorizer.ACLIdentityType.ANONYMOUS);

RequestContext ctxListWithoutSessionPolicy = builder
    .setAclRights(IAccessAuthorizer.ACLType.LIST)
    .build();

RequestContext ctxReadWithoutSessionPolicy = builder
    .setAclRights(IAccessAuthorizer.ACLType.READ)
    .build();

RequestContext ctxListWithSessionPolicy = builder
    .setAclRights(IAccessAuthorizer.ACLType.LIST)
    .setRecursiveAccessCheck(false)
    .setSessionPolicy(sessionPolicy)
    .build();

RequestContext ctxReadWithoutSessionPolicy = builder
    .setAclRights(IAccessAuthorizer.ACLType.READ)
    .setRecursiveAccessCheck(false)
    .setSessionPolicy(sessionPolicy)
    .build();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the inputs, I realized that as soon as you pointed it out.
Went for a quick fix and missed out on the basics.

Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>
@spacemonkd
Copy link
Copy Markdown
Author

Tests have passed:

17:36:01.568 [main] DEBUG org.apache.ranger.plugin.policyengine.RangerPolicyEngineImpl - <== RangerPolicyEngineImpl.evaluateResourceAuditPolicies(request=RangerAccessRequestImpl={resource={RangerResourceImpl={ownerUser={null} elements={role=role1; } }} accessType={assume_role} user={user2} userGroups={} userRoles={} accessTime={null} clientIPAddress={null} forwardedAddresses={} remoteIPAddress={null} clientType={null} action={null} requestData={null} sessionId={null} resourceMatchingScope={SELF} resourceElementMatchingScopes={{}} clusterName={} clusterType={} inlinePolicy={null} context={ISANYACCESS={null} token:USER={user2} _GDS_RESULT={null} ISREQUESTPREPROCESSED={true} RESOURCE_ZONE_NAMES={null} } }, result=RangerAccessResult={isAccessDetermined={true} isAllowed={false} isAuditedDetermined={false} isAudited={false} auditLogId={null} policyType={0} policyId={-1} zoneName={null} auditPolicyId={100} policyVersion={null} evaluatedPoliciesCount={1} reason={null} additionalInfo={}}): ret=false
17:36:01.568 [main] DEBUG org.apache.ranger.plugin.policyengine.RangerPolicyEngineImpl - <== RangerPolicyEngineImpl.evaluateAuditPolicies(result=RangerAccessResult={isAccessDetermined={true} isAllowed={false} isAuditedDetermined={true} isAudited={true} auditLogId={null} policyType={0} policyId={-1} zoneName={null} auditPolicyId={100} policyVersion={null} evaluatedPoliciesCount={1} reason={null} additionalInfo={}})
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 13.481 s - in org.apache.ranger.authorization.ozone.authorizer.TestRangerOzoneAuthorizer

@pradeepagrawal8184 @mneethiraj could you help with reviewing this PR as well?
@adoroszlai could you take another look now?

Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @spacemonkd for the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants