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

Fix bug where ccompat and core api would report different compatibility mode values #5251

Open
wants to merge 1 commit into
base: 2.6.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
import io.apicurio.registry.metrics.health.liveness.ResponseErrorLivenessCheck;
import io.apicurio.registry.metrics.health.readiness.ResponseTimeoutReadinessCheck;
import io.apicurio.registry.model.GA;
import io.apicurio.registry.rules.RulesProperties;
import io.apicurio.registry.rules.compatibility.CompatibilityLevel;
import io.apicurio.registry.storage.RuleNotFoundException;
import io.apicurio.registry.storage.dto.RuleConfigurationDto;
import io.apicurio.registry.types.RuleType;
import jakarta.inject.Inject;
import jakarta.interceptor.Interceptors;

import java.util.Optional;
Expand All @@ -45,6 +47,9 @@
@Logged
public class ConfigResourceImpl extends AbstractResource implements ConfigResource {

@Inject
RulesProperties rulesProperties;

private CompatibilityLevelParamDto getCompatibilityLevel(Supplier<String> supplyLevel) {
try {
// We're assuming the configuration == compatibility level
Expand All @@ -55,26 +60,27 @@ private CompatibilityLevelParamDto getCompatibilityLevel(Supplier<String> supply
)
).get().name());
} catch (RuleNotFoundException ex) {
return new CompatibilityLevelParamDto(CompatibilityLevelDto.Level.NONE.name());
var compatRuleDto = rulesProperties.getDefaultGlobalRuleConfiguration(RuleType.COMPATIBILITY);
if (compatRuleDto != null) {
// Fallback to the default global rule
return new CompatibilityLevelParamDto(compatRuleDto.getConfiguration());
} else {
// Fallback to NONE if no default rule is found
return new CompatibilityLevelParamDto(CompatibilityLevelDto.Level.NONE.name());
}
}
}

private <X extends Exception> void updateCompatibilityLevel(CompatibilityLevelDto.Level level,
Consumer<RuleConfigurationDto> updater,
Runnable deleter) throws X {
if (level == CompatibilityLevelDto.Level.NONE) {
// delete the rule
deleter.run();
} else {
String levelString = level.getStringValue();
try {
CompatibilityLevel.valueOf(levelString);
} catch (IllegalArgumentException ex) {
throw new IllegalArgumentException("Illegal compatibility level: " + levelString);
}
updater.accept(RuleConfigurationDto.builder()
.configuration(levelString).build()); // TODO config should take CompatibilityLevel as param
Consumer<RuleConfigurationDto> updater) throws X {
String levelString = level.getStringValue();
try {
CompatibilityLevel.valueOf(levelString);
} catch (IllegalArgumentException ex) {
throw new IllegalArgumentException("Illegal compatibility level: " + levelString);
}
updater.accept(RuleConfigurationDto.builder()
.configuration(levelString).build()); // TODO config should take CompatibilityLevel as param
}

@Override
Expand All @@ -94,8 +100,8 @@ public CompatibilityLevelDto updateGlobalCompatibilityLevel(CompatibilityLevelDt
} else {
storage.updateGlobalRule(RuleType.COMPATIBILITY, dto);
}
},
() -> storage.deleteGlobalRule(RuleType.COMPATIBILITY));
}
);
return request;
}

Expand All @@ -112,14 +118,8 @@ public CompatibilityLevelDto updateSubjectCompatibilityLevel(String subject, Com
} else {
storage.updateArtifactRule(ga.getGroupId(), ga.getArtifactId(), RuleType.COMPATIBILITY, dto);
}
},
() -> {
try {
storage.deleteArtifactRule(ga.getGroupId(), ga.getArtifactId(), RuleType.COMPATIBILITY);
} catch (RuleNotFoundException e) {
//Ignore, fail only when the artifact is not found
}
});
}
);
return request;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package io.apicurio.registry.ccompat.rest;

import io.apicurio.registry.AbstractResourceTestBase;
import io.apicurio.registry.ccompat.dto.CompatibilityLevelDto;
import io.apicurio.registry.ccompat.dto.CompatibilityLevelParamDto;
import io.apicurio.registry.rest.v2.beans.Rule;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
import jakarta.enterprise.inject.Typed;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;

import static io.apicurio.registry.noprofile.ccompat.rest.CCompatTestConstants.CONFIG_NONE;
import static io.restassured.RestAssured.given;
import static org.junit.jupiter.api.Assertions.assertEquals;

@QuarkusTest
@Typed(CCompatCompatibilityConfigTest.class)
@TestProfile(ForwardCompatModeProfile.class)
public class CCompatCompatibilityConfigTest extends AbstractResourceTestBase {

@NotNull
public String getCcompatBasePath() {
return "/ccompat/v7";
}

@NotNull
public String getCoreBasePath() {
return "/registry/v2";
}

CompatibilityLevelDto.Level getCompatibilityLevelFromCoreApi() {
var respCore = given()
.when()
.contentType(ContentTypes.JSON)
.get(getCoreBasePath() + "/admin/rules/COMPATIBILITY")
.then()
.statusCode(200)
.extract().as(Rule.class);
return CompatibilityLevelDto.Level.valueOf(respCore.getConfig());
}

CompatibilityLevelDto.Level getCompatibilityLevelFromCcompatApi() {
var respCcompat = given()
.when()
.contentType(ContentTypes.COMPAT_SCHEMA_REGISTRY_STABLE_LATEST)
.get(getCcompatBasePath() + "/config/")
.then()
.statusCode(200)
.extract().as(CompatibilityLevelParamDto.class);
return CompatibilityLevelDto.Level.valueOf(respCcompat.getCompatibilityLevel());
}

/**
* Test that the default global compatibility level is the same, no matter if we use
* the core or ccompat endpoint for retrieving its value.
*/
@Test
public void testConfigEndpoints() {
// make sure that the default global "FORWARD" compatibility level is returned by both endpoints
assertEquals(CompatibilityLevelDto.Level.FORWARD, getCompatibilityLevelFromCoreApi());
assertEquals(getCompatibilityLevelFromCoreApi(), getCompatibilityLevelFromCcompatApi());

// change the compatibility level and check that it is the same in both endpoints
given()
.when()
.contentType(ContentTypes.COMPAT_SCHEMA_REGISTRY_STABLE_LATEST)
.body(CONFIG_NONE)
.put(getCcompatBasePath() + "/config")
.then()
.statusCode(200);

assertEquals(CompatibilityLevelDto.Level.NONE, getCompatibilityLevelFromCoreApi());
assertEquals(getCompatibilityLevelFromCoreApi(), getCompatibilityLevelFromCcompatApi());
}

@Override
protected void deleteGlobalRules(int expectedDefaultRulesCount) {
// Do nothing... (we want to keep the default global rules)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package io.apicurio.registry.ccompat.rest;

import io.quarkus.test.junit.QuarkusTestProfile;

import java.util.Map;

public class ForwardCompatModeProfile implements QuarkusTestProfile {
@Override
public Map<String, String> getConfigOverrides() {
return Map.of("registry.rules.global.compatibility", "FORWARD",
"registry.auth.enabled", "false");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public class CCompatTestConstants {

public static final String CONFIG_BACKWARD = "{\"compatibility\": \"BACKWARD\"}";

public static final String CONFIG_NONE = "{\"compatibility\": \"NONE\"}";

public static final String VALID_AVRO_SCHEMA = "{\"schema\": \"{\\\"type\\\": \\\"record\\\",\\\"name\\\": \\\"myrecord1\\\",\\\"fields\\\": [{\\\"name\\\": \\\"foo1\\\",\\\"type\\\": \\\"string\\\"}]}\"}";

public static final String SCHEMA_INVALID_WRAPPED = "{\"schema\":\"{\\\"type\\\": \\\"bloop\\\"}\"}";
Expand Down
Loading