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

RHCLOUD-37430 | fix: integrations are not being returned sometimes #3256

Merged
merged 4 commits into from
Feb 3, 2025
Merged

RHCLOUD-37430 | fix: integrations are not being returned sometimes #3256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d2128d2
fix: integrations are not being returned sometimes
MikelAlejoBR Jan 28, 2025
9f3f2c3
feature: implement post-filtering methods for Kessel
MikelAlejoBR Jan 28, 2025
7b43dfe
refactor: remove debugging log traces
MikelAlejoBR Jan 28, 2025
38464ad
feature: allow configuring the limit when looking up resources
MikelAlejoBR Jan 31, 2025
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
114 changes: 77 additions & 37 deletions backend/src/main/java/com/redhat/cloud/notifications/auth/kessel/KesselAuthorization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.project_kessel.api.relations.v1beta1.LookupResourcesResponse;
import org.project_kessel.api.relations.v1beta1.ObjectReference;
import org.project_kessel.api.relations.v1beta1.ObjectType;
import org.project_kessel.api.relations.v1beta1.RequestPagination;
import org.project_kessel.api.relations.v1beta1.SubjectReference;
import org.project_kessel.relations.client.CheckClient;
import org.project_kessel.relations.client.LookupClient;
Expand Down Expand Up @@ -137,7 +138,8 @@ public void hasPermissionOnResource(final SecurityContext securityContext, final

/**
* Looks up the integrations the security context's subject has the given
* permission for.
* permission for. Useful for when we want to "pre-filter" the integrations
* the principal has authorization for.
* @param securityContext the security context holding the subject's
* identity.
* @param integrationPermission the integration's permission we want to use
Expand All @@ -148,43 +150,66 @@ public Set<UUID> lookupAuthorizedIntegrations(final SecurityContext securityCont
// Identify the subject.
final RhIdentity identity = SecurityContextUtil.extractRhIdentity(securityContext);

// Build the lookup request for Kessel.
final LookupResourcesRequest request = this.buildLookupResourcesRequest(identity, integrationPermission);

Log.tracef("[identity: %s][permission: %s][resource_type: %s] Payload for the resource lookup: %s", identity, integrationPermission, ResourceType.INTEGRATION, request);

// Measure the time it takes to perform the lookup operation.
final Timer.Sample lookupTimer = Timer.start(this.meterRegistry);

// Call Kessel.
final Iterator<LookupResourcesResponse> responses;
try {
responses = this.lookupClient.lookupResources(request);
} catch (final Exception e) {
Log.errorf(
e,
"[identity: %s][permission: %s][resource_type: %s] Runtime error when querying Kessel for integration resources with request payload: %s",
identity, integrationPermission, ResourceType.INTEGRATION, request
);
meterRegistry.counter(KESSEL_METRICS_LOOKUP_RESOURCES_COUNTER_NAME, Tags.of(COUNTER_TAG_REQUEST_RESULT, COUNTER_TAG_FAILURES)).increment();

throw e;
} finally {
// Stop the timer.
lookupTimer.stop(this.meterRegistry.timer(KESSEL_METRICS_LOOKUP_RESOURCES_TIMER_NAME, Tags.of(KESSEL_METRICS_TAG_PERMISSION_KEY, integrationPermission.getKesselPermissionName(), Constants.KESSEL_METRICS_TAG_RESOURCE_TYPE_KEY, ResourceType.INTEGRATION.name())));
}

meterRegistry.counter(KESSEL_METRICS_LOOKUP_RESOURCES_COUNTER_NAME, Tags.of(COUNTER_TAG_REQUEST_RESULT, COUNTER_TAG_SUCCESSES)).increment();

// Process the incoming responses.
// Prepare the set of UUIDs we are going to receive from Kessel.
final Set<UUID> uuids = new HashSet<>();
while (responses.hasNext()) {
final LookupResourcesResponse response = responses.next();

Log.tracef("[identity: %s][permission: %s][resource_type: %s] Received payload for the resource lookup: %s", identity, integrationPermission, ResourceType.INTEGRATION, response);

uuids.add(UUID.fromString(response.getResource().getId()));
}
// Every response coming from Kessel has a continuation token. In order
// to know when to stop querying for resources, we need to keep track
// of the old continuation token and the one from the latest received
// element. Once the old one and the new one are the same, we don't
// have to keep querying for more resources.
String continuationToken;
String newContinuationToken = "";
do {
// Replace the old continuation token with the new one. This way
// the token gets used
continuationToken = newContinuationToken;

// Build the lookup request for Kessel.
final LookupResourcesRequest request = this.buildLookupResourcesRequest(identity, integrationPermission, continuationToken);

Log.tracef("[identity: %s][permission: %s][resource_type: %s] Payload for the resource lookup: %s", identity, integrationPermission, ResourceType.INTEGRATION, request);

// Make the request to Kessel.
final Iterator<LookupResourcesResponse> responses;
try {
responses = this.lookupClient.lookupResources(request);
} catch (final Exception e) {
Log.errorf(
e,
"[identity: %s][permission: %s][resource_type: %s] Runtime error when querying Kessel for integration resources with request payload: %s",
identity, integrationPermission, ResourceType.INTEGRATION, request
);

// Increment the errors counter and stop the timer in case of
// an error.
this.meterRegistry.counter(KESSEL_METRICS_LOOKUP_RESOURCES_COUNTER_NAME, Tags.of(COUNTER_TAG_REQUEST_RESULT, COUNTER_TAG_FAILURES)).increment();
lookupTimer.stop(this.meterRegistry.timer(KESSEL_METRICS_LOOKUP_RESOURCES_TIMER_NAME, Tags.of(KESSEL_METRICS_TAG_PERMISSION_KEY, integrationPermission.getKesselPermissionName(), Constants.KESSEL_METRICS_TAG_RESOURCE_TYPE_KEY, ResourceType.INTEGRATION.name())));

throw e;
}

// Iterate over the incoming results.
while (responses.hasNext()) {
final LookupResourcesResponse response = responses.next();

Log.tracef("[identity: %s][permission: %s][resource_type: %s] Received payload for the resource lookup: %s", identity, integrationPermission, ResourceType.INTEGRATION, response);

uuids.add(UUID.fromString(response.getResource().getId()));

// Update the continuation token every time, to make sure we
// grab the last streamed element's continuation token.
newContinuationToken = response.getPagination().getContinuationToken();
}
g-duval marked this conversation as resolved.
Show resolved Hide resolved

meterRegistry.counter(KESSEL_METRICS_LOOKUP_RESOURCES_COUNTER_NAME, Tags.of(COUNTER_TAG_REQUEST_RESULT, COUNTER_TAG_SUCCESSES)).increment();
} while (!continuationToken.equals(newContinuationToken));

// Stop the timer.
lookupTimer.stop(this.meterRegistry.timer(KESSEL_METRICS_LOOKUP_RESOURCES_TIMER_NAME, Tags.of(KESSEL_METRICS_TAG_PERMISSION_KEY, integrationPermission.getKesselPermissionName(), Constants.KESSEL_METRICS_TAG_RESOURCE_TYPE_KEY, ResourceType.INTEGRATION.name())));

return uuids;
}
Expand Down Expand Up @@ -258,11 +283,14 @@ protected CheckRequest buildCheckRequest(final RhIdentity identity, final Kessel
* @param identity the subject's identity.
* @param kesselPermission the permission we want to check against the
* subject's integrations.
* @param continuationToken the token that will resume fetching resources
* from Kessel from the last point we left off.
* @return a built lookup request that aims at finding integrations for the
* given subject.
*/
protected LookupResourcesRequest buildLookupResourcesRequest(final RhIdentity identity, final KesselPermission kesselPermission) {
return LookupResourcesRequest.newBuilder()
protected LookupResourcesRequest buildLookupResourcesRequest(final RhIdentity identity, final KesselPermission kesselPermission, final String continuationToken) {
// Build the regular query.
final LookupResourcesRequest.Builder requestBuilder = LookupResourcesRequest.newBuilder()
.setSubject(
SubjectReference.newBuilder()
.setSubject(
Expand All @@ -271,8 +299,20 @@ protected LookupResourcesRequest buildLookupResourcesRequest(final RhIdentity id
.setId(getUserId(identity))
).build()
).setRelation(kesselPermission.getKesselPermissionName())
.setResourceType(ResourceType.INTEGRATION.getKesselObjectType())
.build();
.setResourceType(ResourceType.INTEGRATION.getKesselObjectType());

// Include the continuation token in the request to resume fetching
// resources right where we last left off.
if (continuationToken != null && !continuationToken.isBlank()) {
requestBuilder.setPagination(
RequestPagination.newBuilder()
.setContinuationToken(continuationToken)
.setLimit(Integer.MAX_VALUE)
g-duval marked this conversation as resolved.
Show resolved Hide resolved
.build()
);
}

return requestBuilder.build();
}

private String getUserId(RhIdentity identity) {
Expand Down
8 changes: 4 additions & 4 deletions backend/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,16 @@ quarkus.unleash.url=http://localhost:4242
quarkus.log.category."com.redhat.cloud.notifications.routers.internal.kessel.KesselAssetsMigrationService".min-level=TRACE
quarkus.log.category."com.redhat.cloud.notifications.auth.kessel".min-level=TRACE

inventory-api.authn.client.id=insights-notifications
inventory-api.authn.client.id=svc-test
inventory-api.authn.client.issuer=http://localhost:8084/realms/redhat-external
inventory-api.authn.client.secret=development-value-123
inventory-api.authn.client.secret=h91qw8bPiDj9R6VSORsI5TYbceGU5PMH
inventory-api.authn.mode=oidc-client-credentials
inventory-api.is-secure-clients=false
inventory-api.target-url=${clowder.endpoints.kessel-inventory-api:localhost:9081}

relations-api.authn.client.id=insights-notifications
relations-api.authn.client.id=svc-test
relations-api.authn.client.issuer=http://localhost:8084/realms/redhat-external
relations-api.authn.client.secret=development-value-123
Comment on lines -112 to -121
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the default values when launching the Kessel's Inventory API with the authentication enabled: https://github.com/project-kessel/inventory-api/blob/e0fc00b00af6af4977f8b4c66230a34f850da4a2/scripts/get-token.sh

In order to simplify things, I think it's just better to leave the credentials like this by default. It eases development and debugging, and since the credentials are public facing, I think it is just safe to have them here.

relations-api.authn.client.secret=h91qw8bPiDj9R6VSORsI5TYbceGU5PMH
relations-api.authn.mode=oidc-client-credentials
relations-api.is-secure-clients=false
relations-api.target-url=${clowder.endpoints.kessel-relations-api:localhost:9000}
Expand Down
73 changes: 59 additions & 14 deletions ...end/src/test/java/com/redhat/cloud/notifications/auth/kessel/KesselAuthorizationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.project_kessel.api.relations.v1beta1.LookupResourcesRequest;
import org.project_kessel.api.relations.v1beta1.LookupResourcesResponse;
import org.project_kessel.api.relations.v1beta1.ObjectReference;
import org.project_kessel.api.relations.v1beta1.ResponsePagination;
import org.project_kessel.api.relations.v1beta1.SubjectReference;
import org.project_kessel.relations.client.CheckClient;
import org.project_kessel.relations.client.LookupClient;
Expand Down Expand Up @@ -224,32 +225,69 @@ void testLookupAuthorizedIntegrations() {
// Enable the Kessel back end integration for this test.
Mockito.when(this.backendConfig.isKesselRelationsEnabled(anyString())).thenReturn(true);

// Simulate that Kessel returns a few resource IDs in the response.
// Simulate that Kessel returns a few resources. We set a continuation
// token on each element to simulate Kessel's behavior.
final UUID firstUuid = UUID.randomUUID();
final ObjectReference objectReferenceOne = ObjectReference.newBuilder().setId(firstUuid.toString()).build();
final LookupResourcesResponse lookupResourcesResponseOne = LookupResourcesResponse.newBuilder().setResource(objectReferenceOne).build();
final LookupResourcesResponse lookupResourcesResponseOne = LookupResourcesResponse
.newBuilder()
.setResource(objectReferenceOne)
.setPagination(ResponsePagination.newBuilder().setContinuationToken(UUID.randomUUID().toString()).build())
.build();

final UUID secondUuid = UUID.randomUUID();
final ObjectReference objectReferenceTwo = ObjectReference.newBuilder().setId(secondUuid.toString()).build();
final LookupResourcesResponse lookupResourcesResponseTwo = LookupResourcesResponse.newBuilder().setResource(objectReferenceTwo).build();

final LookupResourcesResponse lookupResourcesResponseTwo = LookupResourcesResponse
.newBuilder()
.setResource(objectReferenceTwo)
.setPagination(ResponsePagination.newBuilder().setContinuationToken(UUID.randomUUID().toString()).build())
.build();

// Set a continuation token in the third resource so that we can
// trigger another loop in the "do-while" loop.
final UUID thirdUuid = UUID.randomUUID();
final ObjectReference objectReferenceThree = ObjectReference.newBuilder().setId(thirdUuid.toString()).build();
final LookupResourcesResponse lookupResourcesResponseThree = LookupResourcesResponse.newBuilder().setResource(objectReferenceThree).build();
final LookupResourcesResponse lookupResourcesResponseThree = LookupResourcesResponse
.newBuilder()
.setResource(objectReferenceThree)
.setPagination(ResponsePagination.newBuilder().setContinuationToken(UUID.randomUUID().toString()).build())
.build();

final UUID fourthUuid = UUID.randomUUID();
final ObjectReference objectReferenceFour = ObjectReference.newBuilder().setId(fourthUuid.toString()).build();
final LookupResourcesResponse lookupResourcesResponseFour = LookupResourcesResponse.newBuilder()
.setResource(objectReferenceFour)
.setPagination(ResponsePagination.newBuilder().setContinuationToken(UUID.randomUUID().toString()).build())
.build();

final UUID fifthUuid = UUID.randomUUID();
final ObjectReference objectReferenceFive = ObjectReference.newBuilder().setId(fifthUuid.toString()).build();
final LookupResourcesResponse lookupResourcesResponseFive = LookupResourcesResponse
.newBuilder()
.setResource(objectReferenceFive)
.setPagination(ResponsePagination.newBuilder().setContinuationToken(UUID.randomUUID().toString()).build())
.build();

// Return the iterator to simulate a stream of incoming results from
// Kessel.
final List<LookupResourcesResponse> lookupResourcesResponses = List.of(lookupResourcesResponseOne, lookupResourcesResponseTwo, lookupResourcesResponseThree);
Mockito.when(this.lookupClient.lookupResources(Mockito.any())).thenReturn(lookupResourcesResponses.iterator());
final List<LookupResourcesResponse> lookupResourcesResponsesFirstLoop = List.of(lookupResourcesResponseOne, lookupResourcesResponseTwo, lookupResourcesResponseThree);
final List<LookupResourcesResponse> lookupResourcesResponsesSecondLoop = List.of(lookupResourcesResponseFour, lookupResourcesResponseFive);
Mockito.when(this.lookupClient.lookupResources(Mockito.any())).thenReturn(lookupResourcesResponsesFirstLoop.iterator(), lookupResourcesResponsesSecondLoop.iterator());

// Call the function under test.
final Set<UUID> result = this.kesselAuthorization.lookupAuthorizedIntegrations(mockedSecurityContext, IntegrationPermission.VIEW);

// Asser that three calls were made to Kessel. Since the last element
// returned from Kessel also brings a continuation token, we are always
// forced to make yet another call to make sure that we didn't leave
// any elements unfetched.
Mockito.verify(this.lookupClient, Mockito.times(2 + 1)).lookupResources(Mockito.any());

// Assert counter values
assertCounterIncrements(0, 0, 1, 0);
assertCounterIncrements(0, 0, 2 + 1, 0);

// Assert that the result is the expected one.
final Set<UUID> expectedUuids = Set.of(firstUuid, secondUuid, thirdUuid);
// Assert that the results contain all the UUIDs from the two calls.
final Set<UUID> expectedUuids = Set.of(firstUuid, secondUuid, thirdUuid, fourthUuid, fifthUuid);

result.forEach(r -> Assertions.assertTrue(expectedUuids.contains(r), String.format("UUID \"%s\" not present in the expected UUIDs", r)));
}
Expand Down Expand Up @@ -330,12 +368,13 @@ public String toString() {
*/
@Test
void testBuildLookupResourcesRequest() {
record TestCase(RhIdentity identity, KesselPermission permission) {
record TestCase(RhIdentity identity, KesselPermission permission, String continuationToken) {
@Override
public String toString() {
return "TestCase{" +
"identity='" + this.identity + '\'' +
", kesselPermission='" + this.permission + '\'' +
", continuationToken='" + this.continuationToken + '\'' +
'}';
}
}
Expand All @@ -352,13 +391,13 @@ public String toString() {

// Loop through the supported identities.
final List<TestCase> testCases = List.of(
new TestCase(userIdentity, IntegrationPermission.VIEW),
new TestCase(serviceAccountIdentity, IntegrationPermission.VIEW)
new TestCase(userIdentity, IntegrationPermission.VIEW, null),
new TestCase(serviceAccountIdentity, IntegrationPermission.VIEW, "cont-token-123")
);

for (final TestCase tc : testCases) {
// Call the function under test.
final LookupResourcesRequest lookupResourcesRequest = this.kesselAuthorization.buildLookupResourcesRequest(tc.identity(), tc.permission());
final LookupResourcesRequest lookupResourcesRequest = this.kesselAuthorization.buildLookupResourcesRequest(tc.identity(), tc.permission(), tc.continuationToken);

// Make sure the request was built appropriately.
final SubjectReference subjectReference = lookupResourcesRequest.getSubject();
Expand All @@ -368,6 +407,12 @@ public String toString() {
Assertions.assertEquals(tc.permission().getKesselPermissionName(), lookupResourcesRequest.getRelation(), String.format("unexpected relation obtained on test case: %s", tc));

Assertions.assertEquals(ResourceType.INTEGRATION.getKesselObjectType(), lookupResourcesRequest.getResourceType(), String.format("unexpected resource type obtained on test case: %s", tc));

if (tc.continuationToken() == null) {
Assertions.assertEquals("", lookupResourcesRequest.getPagination().getContinuationToken(), String.format("the continuation token should have been an empty string when none is given when building the request: %s", tc));
} else {
Assertions.assertEquals(tc.continuationToken, lookupResourcesRequest.getPagination().getContinuationToken(), String.format("unexpected continuation token obtained on test case: %s", tc));
}
}
}

Expand Down
Loading