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

feat: Policy Cache mechanism improvements[staging] #2736

Merged
merged 17 commits into from
Jan 10, 2024
Merged

Conversation

sumandas0
Copy link

Policy Cache mechanism improvements

We are enhancing the way we build policy caches within the authorizer to improve authorization evaluation and reduce the number of calls made to Keycloak. This includes additional enhancements as well.
Instead of making numerous API calls to Keycloak, we will create custom views in Postgres tailored to our specific requirements. We will then fetch the necessary information from Heracles and use it to build the cache

More details: https://www.notion.so/atlanhq/Policy-Cache-mechanism-improvements-028fe879ed2a4604ae880a263726b2fe?pvs=4

JIRA: https://atlanhq.atlassian.net/browse/PLT-524

Test Cases: https://atlanhq.atlassian.net/browse/PLT-524?focusedCommentId=50549

Type of change

  • Bug fix (fixes an issue)
  • New Enhancement (adds functionality)

Development

  • Lint rules pass locally
  • Application changes have been tested thoroughly
  • Automated tests covering modified code pass

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

Code review

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached as necessary
  • "Ready for review" label attached and reviewers assigned
  • Changes have been reviewed by at least one other contributor
  • Pull request linked to task tracker where applicable

@sumandas0 sumandas0 changed the base branch from master to staging January 9, 2024 11:03
@sumandas0 sumandas0 changed the title feat: Policy Cache mechanism improvements feat: Policy Cache mechanism improvements[staging] Jan 9, 2024
Copy link

@n5nk n5nk left a comment

Choose a reason for hiding this comment

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

refresh the branch with master, i see few unrelated commits.

public List<UserRepresentation> getUsersMappings(int start, int size) throws AtlasBaseException {
String[] columns = {"roles", "groups"};
List<HeraclesUserViewRepresentation> views = HERACLES.getUsersMappings(start, size, HeraclesUserViewRepresentation.sortBy, columns).body();
List<UserRepresentation> userRepresentations = views.stream().map(x -> {
Copy link

Choose a reason for hiding this comment

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

nit: No need for assignment userRepresentations, just return

protected List<String> roles;
protected List<String> groups;

public static String sortBy = "username";
Copy link

Choose a reason for hiding this comment

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

I dont see a usage for this variable sortBy

Copy link
Author

Choose a reason for hiding this comment

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

protected List<String> roles;
protected List<String> groups;

public static String sortBy = "name";
Copy link

Choose a reason for hiding this comment

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

I dont see a usage for this variable sortBy, and its referring to name , missed deleting ?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Using it here ^

Choose a reason for hiding this comment

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

rename to something like SORT_KEY_BY_NAME..

@@ -100,6 +100,11 @@ Config getProdConfig() throws AtlasException {
.setClientName(ATLAS_METASTORE_SERVICE)
.setReadMode(ReadMode.MASTER_SLAVE)
.setCheckSentinelsList(false)
.setKeepAlive(true)
Copy link

Choose a reason for hiding this comment

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

refresh the branch with master, i see unrelated commits like these

Copy link
Author

Choose a reason for hiding this comment

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

Done

@sumandas0 sumandas0 changed the base branch from staging to master January 10, 2024 10:17
@sumandas0 sumandas0 changed the base branch from master to staging January 10, 2024 10:17
protected List<String> roles;
protected List<String> groups;

public static String sortBy = "name";

Choose a reason for hiding this comment

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

rename to something like SORT_KEY_BY_NAME..

@@ -108,7 +108,8 @@ public enum AtlasConfiguration {

PERSONA_POLICY_ASSET_MAX_LIMIT("atlas.persona.policy.asset.maxlimit", 1000),
ENABLE_KEYCLOAK_TOKEN_INTROSPECTION("atlas.canary.keycloak.token-introspection", false),
KEYCLOAK_ADMIN_CLIENT_PAGINATION_SIZE("atlas.keycloak.admin.resource-pagination-size", 1500);
HERACLES_CLIENT_PAGINATION_SIZE("atlas.heracles.admin.resource-pagination-size", 100),
HERACLES_API_SERVER_URL("atlas.heracles.api.server.url", "http://heracles-service.heracles.svc.cluster.local");

Choose a reason for hiding this comment

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

rename to atlas.heracles.api.service.url

Copy link
Author

Choose a reason for hiding this comment

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

Done

@sumandas0 sumandas0 merged commit 1b9e926 into staging Jan 10, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants