Skip to content

Commit

Permalink
Adjusted tests to code changes | WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
@druminski
druminski committed Jun 7, 2024
1 parent 1e1dc9d commit 43be8fc
Show file tree
Hide file tree
Showing 12 changed files with 164 additions and 85 deletions.
7 changes: 4 additions & 3 deletions envoy-control-core/src/main/java/pl/allegro/tech/servicemesh/envoycontrol/SimpleCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public Watch createWatch(
XdsRequest request,
Set<String> knownResourceNames,
Consumer<Response> responseConsumer) {
return createWatch(ads, request, knownResourceNames, responseConsumer, false);
return createWatch(ads, request, knownResourceNames, responseConsumer, false, false);
}

/**
Expand All @@ -106,7 +106,8 @@ public Watch createWatch(
XdsRequest request,
Set<String> knownResourceNames,
Consumer<Response> responseConsumer,
boolean hasClusterChanged) {
boolean hasClusterChanged,
boolean allowDefaultEmptyEdsUpdate) {
Resources.ResourceType requestResourceType = request.getResourceType();
Preconditions.checkNotNull(requestResourceType, "unsupported type URL %s",
request.getTypeUrl());
Expand All @@ -123,7 +124,7 @@ public Watch createWatch(
U snapshot = snapshots.get(group);
String version = snapshot == null ? "" : snapshot.version(requestResourceType, request.getResourceNamesList());

Watch watch = new Watch(ads, request, responseConsumer);
Watch watch = new Watch(ads, allowDefaultEmptyEdsUpdate, request, responseConsumer);

if (snapshot != null) {
Set<String> requestedResources = ImmutableSet.copyOf(request.getResourceNamesList());
Expand Down
12 changes: 7 additions & 5 deletions ...ore/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/GroupChangeWatcher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,16 @@ internal class GroupChangeWatcher(

override fun createWatch(
ads: Boolean,
request: XdsRequest,
knownResourceNames: MutableSet<String>,
responseConsumer: Consumer<Response>,
hasClusterChanged: Boolean
request: XdsRequest?,
knownResourceNames: MutableSet<String>?,
responseConsumer: Consumer<Response>?,
hasClusterChanged: Boolean,
allowDefaultEmptyEdsUpdate: Boolean
): Watch {
val oldGroups = cache.groups()

val watch = cache.createWatch(ads, request, knownResourceNames, responseConsumer, hasClusterChanged)
val watch = cache.createWatch(ads, request, knownResourceNames, responseConsumer, hasClusterChanged,
allowDefaultEmptyEdsUpdate)
val groups = cache.groups()
metrics.setCacheGroupsCount(groups.size)
if (oldGroups != groups) {
Expand Down
18 changes: 9 additions & 9 deletions ...trol-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ fun Value.toIncomingEndpoint(properties: SnapshotProperties): IncomingEndpoint {
val oauth = properties.let { this.field("oauth")?.toOAuth(it) }

return when {
paths.isNotEmpty() -> IncomingEndpoint(paths, "", PathMatchingType.PATHS, methods, clients, unlistedClientsPolicy, oauth)
paths.isNotEmpty() -> IncomingEndpoint(paths, "", PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
path != null -> IncomingEndpoint(paths, path, PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
pathPrefix != null -> IncomingEndpoint(
paths, pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, unlistedClientsPolicy, oauth
Expand Down Expand Up @@ -422,15 +422,15 @@ fun Value.toIncomingRateLimitEndpoint(): IncomingRateLimitEndpoint {
}
}

fun isMoreThanOnePropertyDefined(vararg properties: Any?): Boolean = properties.filter {
if (it != null) {
if (it is Set<*>) {
return it.isNotEmpty()
}
return true
fun isMoreThanOnePropertyDefined(vararg properties: Any?): Boolean = countNonNullAndNotEmptyProperties(properties.toList()) > 1

private fun countNonNullAndNotEmptyProperties(props: List<Any?>): Int = props.filterNotNull().count {
if (it is Set<*>) {
it.isNotEmpty()
} else {
true
}
return false
}.count() > 1
}

private fun Value?.toIncomingTimeoutPolicy(): Incoming.TimeoutPolicy {
val idleTimeout: Duration? = this?.field("idleTimeout")?.toDuration()
Expand Down
74 changes: 47 additions & 27 deletions ...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ class JwtFilterFactory(
.build()

private fun createRules(endpoints: List<IncomingEndpoint>): Set<RequirementRule> {
return endpoints.flatMap(this::createRuleForEndpoint).toSet()
return endpoints.flatMap(this::createRulesForEndpoint).toSet()
}

private fun createRuleForEndpoint(endpoint: IncomingEndpoint): List<RequirementRule> {
private fun createRulesForEndpoint(endpoint: IncomingEndpoint): Set<RequirementRule> {
val providers = mutableSetOf<String>()

if (endpoint.oauth != null) {
Expand All @@ -97,15 +97,45 @@ class JwtFilterFactory(
providers.addAll(endpoint.clients.filter { it.name in clientToOAuthProviderName.keys }
.mapNotNull { clientToOAuthProviderName[it.name] })

return if (providers.isNotEmpty()) {
if (providers.isEmpty()) {
return emptySet()
}

return if (endpoint.paths.isNotEmpty()) {
endpoint.paths.map {
requirementRuleWithPathMatching(it, endpoint.pathMatchingType, endpoint.methods, providers)
} + requirementRuleWithPathMatching(endpoint.path, endpoint.pathMatchingType, endpoint.methods, providers)
requirementRuleWithURITemplateMatching(it, endpoint.methods, providers)
}.toSet()
} else {
emptyList()
setOf(requirementRuleWithPathMatching(endpoint.path, endpoint.pathMatchingType, endpoint.methods, providers))
}
}

private fun requirementRuleWithURITemplateMatching(
pathGlobPattern: String,
methods: Set<String>,
providers: MutableSet<String>
): RequirementRule {
val pathMatching = RouteMatch.newBuilder().setPathMatchPolicy(
TypedExtensionConfig.newBuilder()
.setName("envoy.path.match.uri_template.uri_template_matcher")
.setTypedConfig(
Any.pack(
UriTemplateMatchConfig.newBuilder()
.setPathTemplate(pathGlobPattern)
.build()
)
).build()
)
if (methods.isNotEmpty()) {
pathMatching.addHeaders(createHeaderMatcherBuilder(methods))
}

return RequirementRule.newBuilder()
.setMatch(pathMatching)
.setRequires(createJwtRequirement(providers))
.build()
}

private fun requirementRuleWithPathMatching(
path: String,
pathMatchingType: PathMatchingType,
Expand All @@ -122,36 +152,26 @@ class JwtFilterFactory(
).build()
)
}
pathMatching.setPathMatchPolicy(
TypedExtensionConfig.newBuilder()
.setName("envoy.path.match.uri_template.uri_template_matcher")
.setTypedConfig(
Any.pack(
UriTemplateMatchConfig.newBuilder()
.setPathTemplate(path)
.build()
)
).build()
)
if (methods.isNotEmpty()) {
val methodsRegexp = methods.joinToString("|")
val headerMatcher = HeaderMatcher.newBuilder()
.setName(":method")
.setSafeRegexMatch(
RegexMatcher.newBuilder().setRegex(methodsRegexp).setGoogleRe2(
RegexMatcher.GoogleRE2.getDefaultInstance()
).build()
)
pathMatching.addHeaders(headerMatcher)
pathMatching.addHeaders(createHeaderMatcherBuilder(methods))
}

return RequirementRule.newBuilder()
.setMatch(pathMatching)
.setMatch()
.setRequires(createJwtRequirement(providers))
.build()
}

private fun createHeaderMatcherBuilder(methods: Set<String>): HeaderMatcher.Builder {
return HeaderMatcher.newBuilder()
.setName(":method")
.setSafeRegexMatch(
RegexMatcher.newBuilder().setRegex(methods.joinToString("|")).setGoogleRe2(
RegexMatcher.GoogleRE2.getDefaultInstance()
).build()
)
}

private val requirementsForProviders: Map<ProviderName, JwtRequirement> =
jwtProviders.keys.associateWith { JwtRequirement.newBuilder().setProviderName(it).build() }

Expand Down
4 changes: 2 additions & 2 deletions ...ech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterPermissions.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.PathMatchingType
class RBACFilterPermissions {
fun createCombinedPermissions(incomingEndpoint: IncomingEndpoint): Permission.Builder {
val permissions = listOfNotNull(
createMethodPermissions(incomingEndpoint),
if (incomingEndpoint.paths.isNotEmpty())
createPathTemplatesPermissionForEndpoint(incomingEndpoint)
else
createPathPermissionForEndpoint(incomingEndpoint),
createMethodPermissions(incomingEndpoint),
)
.map { it.build() }

Expand All @@ -27,7 +27,7 @@ class RBACFilterPermissions {
)
}

private fun createPathTemplatesPermissionForEndpoint(incomingEndpoint: IncomingEndpoint): Permission.Builder {
private fun createPathTemplatesPermissionForEndpoint(incomingEndpoint: IncomingEndpoint): Permission.Builder {
return permission()
.setOrRules(Permission.Set.newBuilder().addAllRules(
incomingEndpoint.paths.map(this::createPathTemplate)))
Expand Down
52 changes: 39 additions & 13 deletions ...ntrol-core/src/test/java/pl/allegro/tech/servicemesh/envoycontrol/v3/SimpleCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ public void invalidNamesListShouldReturnWatcherWithNoResponseInAdsMode() {
.build()),
Collections.emptySet(),
responseTracker,
false);
false,
false
);

assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
}
Expand All @@ -114,7 +116,9 @@ public void invalidNamesListShouldReturnWatcherWithResponseInXdsMode() {
.build()),
Collections.emptySet(),
responseTracker,
false);
false,
false
);

assertThat(watch.isCancelled()).isFalse();
assertThat(responseTracker.responses).isNotEmpty();
Expand All @@ -138,7 +142,9 @@ public void successfullyWatchAllResourceTypesWithSetBeforeWatch() {
.build()),
Collections.emptySet(),
responseTracker,
false);
false,
false
);

assertThat(watch.request().getTypeUrl()).isEqualTo(typeUrl);
assertThat(watch.request().getResourceNamesList()).containsExactlyElementsOf(
Expand Down Expand Up @@ -166,7 +172,9 @@ public void shouldSendEdsWhenClusterChangedButEdsVersionDidnt() {
.build()),
Sets.newHashSet(""),
responseTracker,
true);
true,
false
);

assertThat(watch.request().getTypeUrl()).isEqualTo(Resources.V3.ENDPOINT_TYPE_URL);
assertThat(watch.request().getResourceNamesList()).containsExactlyElementsOf(
Expand Down Expand Up @@ -194,7 +202,9 @@ public void successfullyWatchAllResourceTypesWithSetAfterWatch() {
.build()),
Collections.emptySet(),
responseTracker,
false);
false,
false
);

return new WatchAndTracker(watch, responseTracker);
}));
Expand Down Expand Up @@ -236,7 +246,9 @@ public void successfullyWatchAllResourceTypesWithSetBeforeWatchWithRequestVersio
responseTracker.accept(r);
responseOrderTracker.accept(r);
},
false);
false,
false
);

return new WatchAndTracker(watch, responseTracker);
}))
Expand Down Expand Up @@ -288,7 +300,9 @@ public void successfullyWatchAllResourceTypesWithSetBeforeWatchWithSameRequestVe
.build()),
SNAPSHOT2.resources(typeUrl).keySet(),
responseTracker,
false);
false,
false
);

return new WatchAndTracker(watch, responseTracker);
}));
Expand Down Expand Up @@ -333,7 +347,9 @@ public void successfullyWatchAllResourceTypesWithSetBeforeWatchWithSameRequestVe
.build()),
SNAPSHOT2.resources(typeUrl).keySet(),
responseTracker,
false);
false,
false
);

return new WatchAndTracker(watch, responseTracker);
}));
Expand Down Expand Up @@ -366,7 +382,9 @@ public void setSnapshotWithVersionMatchingRequestShouldLeaveWatchOpenWithoutAddi
.build()),
SNAPSHOT1.resources(typeUrl).keySet(),
responseTracker,
false);
false,
false
);

return new WatchAndTracker(watch, responseTracker);
}));
Expand Down Expand Up @@ -403,7 +421,9 @@ public void watchesAreReleasedAfterCancel() {
.build()),
Collections.emptySet(),
responseTracker,
false);
false,
false
);

return new WatchAndTracker(watch, responseTracker);
}));
Expand Down Expand Up @@ -435,7 +455,9 @@ public void watchIsLeftOpenIfNotRespondedImmediately() {
.build()),
Collections.singleton(ROUTE_NAME),
responseTracker,
false);
false,
false
);

assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
}
Expand Down Expand Up @@ -475,7 +497,9 @@ public void clearSnapshotWithWatches() {
.build()),
Collections.emptySet(),
r -> { },
false);
false,
false
);

// clearSnapshot should fail and the snapshot should be left untouched
assertThat(cache.clearSnapshot(SingleNodeGroup.GROUP)).isFalse();
Expand All @@ -502,7 +526,9 @@ public void groups() {
.build()),
Collections.emptySet(),
r -> { },
false);
false,
false
);

assertThat(cache.groups()).containsExactly(SingleNodeGroup.GROUP);
}
Expand Down
4 changes: 3 additions & 1 deletion ...java/pl/allegro/tech/servicemesh/envoycontrol/v3/SimpleCacheWithMissingEndpointsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ public void missingNamesListShouldReturnWatcherWithResponseInAdsMode() {
.build()),
Collections.emptySet(),
responseTracker,
false);
false,
false
);

assertThatWatchReceivesSnapshotWithMissingResources(new WatchAndTracker(watch, responseTracker), SNAPSHOT_WITH_MISSING_RESOURCES);
}
Expand Down
Loading

0 comments on commit 43be8fc

Please sign in to comment.