diff --git a/providers/flagd/README.md b/providers/flagd/README.md index 3ee76f0c09..4daa06b84e 100644 --- a/providers/flagd/README.md +++ b/providers/flagd/README.md @@ -358,3 +358,42 @@ FlagdOptions options = FlagdOptions.builder() .resolverType(Config.Resolver.IN_PROCESS) .build(); ``` + +## Provider conformance (TCK) + +This provider adopts the [OpenFeature Provider TCK](../../tools/tck/README.md), once per resolver: +`RpcTest`, `InProcessTest` and `FileTest`, all three over the shared `AbstractResolverTest` in +`src/test/java/.../flagd/tck/`. **Read that class before changing any of them.** It records, against +measured behaviour rather than assumption, which capabilities are declared, which are withheld and +why, and every known deviation — that reasoning is the most valuable thing about this adoption and it +lives next to the declaration rather than here. + +```bash +mvn -Ptck -pl providers/flagd -am test # every resolver +mvn -Ptck -pl providers/flagd -am -Dtest=InProcessTest -Dsurefire.failIfNoSpecifiedTests=false test # one +``` + +The root pom's `tck` profile runs only the `tck` package, so `-am` adds no other module's tests. + +**`-Pe2e` does not run these suites.** The root pom excludes every provider's `tck` package from +the default build and from `-Pe2e`, and its `tck` profile runs only that package. This module +excludes just `**/e2e/*.java`, which the `e2e` profile clears. + +**CI runs them in a job of their own**, `Provider TCK` in `ci.yml`, on every pull request. It sits +apart from the `main` matrix so a conformance failure cannot be read as a regression there. Set +`tck.testFailureIgnore` to `true` in this module's pom, or pass `-Dtck.testFailureIgnore=true`, to +report failures without failing the build. + +On `flagd-testbed` v3.11.0, 65 scenarios and 2 skipped in each gRPC mode, and **all 63 pass in both**. +Any failure is a regression. + +The file resolver reads its ruleset off the disk, so `FileTest` has the testbed write it to a +bind-mounted directory (`composeEnvironment()` sets `FLAGS_DIR`) and points the provider at +`allFlags.json` there. It withholds `@stale` on top of `@reinitialization`: the only outage the +control API can stage is stopping flagd's listeners, which a file already on disk does not notice. +`@configuration-change` holds, through polling rather than a stream. + +The two skips are the withheld `@reinitialization` and `@large-integers`, which the Java SDK cannot +ask for. The three scenarios that used to fail on flags the testbed did not serve +(open-feature/flagd-testbed#392) pass from v3.11.0 on, and the one that used to fail in-process +passes since `FlagdCore` stopped narrowing a `Double` with `intValue()` (#1957). diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 063c533342..daf72975a4 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -14,7 +14,11 @@ 0.15.0 - + **/e2e/*.java 1.85.0 @@ -22,6 +26,13 @@ 1.2.28 [3.0.0,4.0.0) + + [0.1.0,) + + false flagd @@ -98,6 +109,17 @@ 5.14.4 test + + + dev.openfeature.contrib.tools + tck + ${tck.version} + test + org.testcontainers testcontainers @@ -250,7 +272,11 @@ e2e - + diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java index 0bb64dd5cb..d38c8339dd 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java @@ -1,6 +1,7 @@ package dev.openfeature.contrib.providers.flagd; import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.PLUGIN_PROPERTY_NAME; @@ -20,6 +21,9 @@ @SelectFile("test-harness/gherkin/config.feature") @ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "summary") @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps.config") +// The TCK ships its own Cucumber ObjectFactory, so two are on the test classpath and Cucumber +// refuses to guess. The e2e suites name the same one; this suite needs it for the same reason. +@ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory") // Config scenarios read System env vars in FlagdOptions.build() and some scenarios also // mutate them. Parallel execution causes env-var races (e.g. FLAGD_PORT=3456 leaking into // a "Default Config" scenario that expects 8015). Since the entire suite runs in <0.4s, diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/AbstractResolverTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/AbstractResolverTest.java new file mode 100644 index 0000000000..f8a9b1b545 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/AbstractResolverTest.java @@ -0,0 +1,129 @@ +package dev.openfeature.contrib.providers.flagd.tck; + +import dev.openfeature.contrib.providers.flagd.Config; +import dev.openfeature.contrib.providers.flagd.FlagdOptions; +import dev.openfeature.contrib.providers.flagd.FlagdProvider; +import dev.openfeature.contrib.tools.tck.BackendEndpoint; +import dev.openfeature.contrib.tools.tck.Capability; +import dev.openfeature.contrib.tools.tck.ContainerizedProviderTckTest; +import dev.openfeature.sdk.FeatureProvider; +import java.io.File; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +/** + * Shared configuration for running the OpenFeature Provider TCK against the flagd provider. + * + *

flagd resolves flags in three quite different ways, and all three are worth conforming: RPC + * evaluates remotely over gRPC, in-process syncs the ruleset and evaluates locally, and file reads + * the ruleset off the disk. All three share a backend stack; the two gRPC modes differ only in + * resolver and port, and {@link FileTest} says what the third needs instead. + */ +abstract class AbstractResolverTest extends ContainerizedProviderTckTest { + + /** A port nothing listens on; an outage on the Compose stack belongs to the control API. */ + private static final int UNAVAILABLE_PORT = 9999; + + /** + * gRPC deadline for a provider that is expected to connect. + * + *

flagd doubles it, and in-process must sync the entire ruleset before it reports + * ready; 5000 was not enough for that. + */ + private static final int CONNECTED_DEADLINE_MS = 15000; + + /** gRPC deadline for a dead port; these scenarios assert that the failure is reported promptly. */ + private static final int UNAVAILABLE_DEADLINE_MS = 1000; + + /** The resolver under test. */ + protected abstract Config.Resolver resolver(); + + /** + * The container-internal port this mode waits on before the first scenario. + * + *

The two gRPC resolvers dial it. {@link FileTest} does not, and names flagd's sync listener + * only because a flagd that is listening is a flagd that has written its flag file. + */ + protected abstract int backendPort(); + + /** + * {@inheritDoc} + * + *

The flagd-testbed's own stack, from the {@code test-harness} submodule the legacy e2e suites + * also run, so the image tag follows the submodule pin instead of a second copy that can drift. + */ + @Override + public File composeFile() { + return new File("test-harness/docker-compose.yaml"); + } + + /** The testbed's compose file names the flagd service {@code flagd}. */ + @Override + public String backendService() { + return "flagd"; + } + + @Override + public List backendPorts() { + return Collections.singletonList(backendPort()); + } + + @Override + public FeatureProvider createProvider(BackendEndpoint endpoint) { + return new FlagdProvider(flagSource(baseOptions().deadline(CONNECTED_DEADLINE_MS), endpoint) + .build()); + } + + @Override + public FeatureProvider createUnavailableProvider() { + return new FlagdProvider(missingFlagSource(baseOptions().deadline(UNAVAILABLE_DEADLINE_MS)) + .build()); + } + + /** + * Points the options at the running stack; a resolver that reads no socket overrides this. + * + * @param options the options being built, with the connected deadline already set + * @param endpoint host and mapped ports of the running stack + * @return the same builder, told where to find the flags + */ + protected FlagdOptions.FlagdOptionsBuilder flagSource( + FlagdOptions.FlagdOptionsBuilder options, BackendEndpoint endpoint) { + return options.host(endpoint.host()).port(endpoint.port(backendPort())); + } + + /** + * Points the options at a flag source that is not there, for the {@code @unavailable} scenarios. + * + * @param options the options being built, with the short deadline already set + * @return the same builder, told to look somewhere nothing answers + */ + protected FlagdOptions.FlagdOptionsBuilder missingFlagSource(FlagdOptions.FlagdOptionsBuilder options) { + return options.host("localhost").port(UNAVAILABLE_PORT); + } + + /** + * {@inheritDoc} + * + *

Everything declarable except {@link Capability#REINITIALIZATION}, which is a fact about the + * provider rather than a defect in it: {@code shutdown()} never clears {@code isInitialized}, so a + * later {@code initialize()} returns at its first check without rebuilding the resolver, the + * {@code shutdownNow()}'d channel or the terminated executors. A shut-down flagd provider is + * terminally shut down. {@link FileTest} narrows this further, for the reason it gives. + * + *

{@link Capability#declarableExcept} and not {@code EnumSet.complementOf}, whose complement + * would include reserved tags the suite refuses at startup. + */ + @Override + public Set capabilities() { + return Capability.declarableExcept(Capability.REINITIALIZATION); + } + + private FlagdOptions.FlagdOptionsBuilder baseOptions() { + return FlagdOptions.builder() + .resolverType(resolver()) + .retryGracePeriod(2) + .retryBackoffMs(500); + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/FileTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/FileTest.java new file mode 100644 index 0000000000..6336d67719 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/FileTest.java @@ -0,0 +1,116 @@ +package dev.openfeature.contrib.providers.flagd.tck; + +import dev.openfeature.contrib.providers.flagd.Config; +import dev.openfeature.contrib.providers.flagd.FlagdOptions; +import dev.openfeature.contrib.tools.tck.BackendEndpoint; +import dev.openfeature.contrib.tools.tck.Capability; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import org.apache.commons.io.FileUtils; + +/** + * Runs the OpenFeature Provider TCK against the flagd provider in file mode. + * + *

This resolver reads its ruleset from a file instead of a socket, so the stack has to put that + * file where this JVM can see it. The testbed's Compose file mounts {@code $FLAGS_DIR} at + * {@code /flags}, where its launchpad writes {@code allFlags.json}, so naming a host directory + * through {@link #composeEnvironment()} is all it takes. + */ +public class FileTest extends AbstractResolverTest { + + /** + * Where the testbed writes its flags and this provider reads them. + * + *

A fixed name under the module's gitignored {@code tmp/}, cleared on the way in, so a crashed + * run leaves one directory behind instead of one more each time. Static because + * {@code beginSuite()} validates the declarations on a throwaway harness with no stack. + */ + private static final Path FLAGS_DIR = flagsDir(); + + /** The file flagd's launchpad combines the ruleset into; see {@code launchpad/pkg/json.go}. */ + private static final String FLAG_FILE = "allFlags.json"; + + /** + * How often to restat the flag file. + * + *

Far below {@code Config.DEFAULT_OFFLINE_POLL_MS}, which is sane for a long-lived provider and + * far too slow for a scenario: polling is the only way this resolver learns of a change, so + * {@code @configuration-change} waits out a whole interval before it can pass. + */ + private static final int POLL_INTERVAL_MS = 100; + + /** + * {@inheritDoc} + * + *

Stated rather than derived, for the reason {@link RpcTest#configuration()} gives. + */ + @Override + public String configuration() { + return "flagd-file"; + } + + @Override + public Config.Resolver resolver() { + return Config.Resolver.FILE; + } + + /** flagd's sync listener; this resolver dials nothing, see {@link AbstractResolverTest}. */ + @Override + protected int backendPort() { + return 8015; + } + + /** {@inheritDoc} */ + @Override + public Map composeEnvironment() { + return Collections.singletonMap("FLAGS_DIR", FLAGS_DIR.toString()); + } + + /** The endpoint is ignored: the ruleset arrives on the filesystem, not over a socket. */ + @Override + protected FlagdOptions.FlagdOptionsBuilder flagSource( + FlagdOptions.FlagdOptionsBuilder options, BackendEndpoint endpoint) { + return options.offlineFlagSourcePath(FLAGS_DIR.resolve(FLAG_FILE).toString()) + .offlinePollIntervalMs(POLL_INTERVAL_MS); + } + + /** A path rather than a closed port, which is what "unreachable" means to this resolver. */ + @Override + protected FlagdOptions.FlagdOptionsBuilder missingFlagSource(FlagdOptions.FlagdOptionsBuilder options) { + return options.offlineFlagSourcePath(FLAGS_DIR.resolve("not-existing").toString()) + .offlinePollIntervalMs(POLL_INTERVAL_MS); + } + + /** + * {@inheritDoc} + * + *

{@link AbstractResolverTest#capabilities()} minus {@link Capability#STALE}. The provider can + * go stale here, but the only outage the control API can stage is stopping flagd's listeners, and + * a file already on disk survives that, so nothing would cause the stale event the scenario waits + * for. + * + *

{@link Capability#CONFIGURATION_CHANGE} is declared: the control API rewrites the files in + * the mounted volume, so the change reaches this provider through {@link #POLL_INTERVAL_MS} + * rather than over a stream. + */ + @Override + public Set capabilities() { + return Capability.declarableExcept(Capability.REINITIALIZATION, Capability.STALE); + } + + private static Path flagsDir() { + Path directory = Paths.get("tmp", "tck-file"); + try { + FileUtils.deleteDirectory(directory.toFile()); + return Files.createDirectories(directory).toAbsolutePath(); + } catch (IOException e) { + throw new UncheckedIOException("Could not prepare the directory the testbed writes its flags to", e); + } + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/InProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/InProcessTest.java new file mode 100644 index 0000000000..3dd2341506 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/InProcessTest.java @@ -0,0 +1,27 @@ +package dev.openfeature.contrib.providers.flagd.tck; + +import dev.openfeature.contrib.providers.flagd.Config; + +/** Runs the OpenFeature Provider TCK against the flagd provider in in-process mode. */ +public class InProcessTest extends AbstractResolverTest { + + /** + * {@inheritDoc} + * + *

Stated rather than derived, for the reason {@link RpcTest#configuration()} gives. + */ + @Override + public String configuration() { + return "flagd-in-process"; + } + + @Override + protected Config.Resolver resolver() { + return Config.Resolver.IN_PROCESS; + } + + @Override + protected int backendPort() { + return 8015; + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/RpcTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/RpcTest.java new file mode 100644 index 0000000000..2a3ebe12ef --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/RpcTest.java @@ -0,0 +1,29 @@ +package dev.openfeature.contrib.providers.flagd.tck; + +import dev.openfeature.contrib.providers.flagd.Config; + +/** Runs the OpenFeature Provider TCK against the flagd provider in RPC mode. */ +public class RpcTest extends AbstractResolverTest { + + /** + * {@inheritDoc} + * + *

Stated rather than derived from the class name, which says only which resolver this is: a + * report is read away from this repository, where {@code rpc} on its own would not say whose RPC + * resolver it was. + */ + @Override + public String configuration() { + return "flagd-rpc"; + } + + @Override + protected Config.Resolver resolver() { + return Config.Resolver.RPC; + } + + @Override + protected int backendPort() { + return 8013; + } +} diff --git a/tools/tck/README.md b/tools/tck/README.md index af301bb64a..ec1851827d 100644 --- a/tools/tck/README.md +++ b/tools/tck/README.md @@ -126,6 +126,7 @@ Overridable on `ContainerizedProviderTckTest`; names and defaults match every la | `backendService()` | `backend` | service hosting both the backend and control API | | `controlPort()` | `8080` | | | `additionalPorts()` | none | `Map>` of extra service to ports, resolved via `BackendEndpoint` by service name | +| `composeEnvironment()` | none | environment for the Compose invocation, for a stack that takes a host path -- a bind-mounted directory the test JVM also reads | | `backendConfiguration()` | `default` | name passed to the control API's `POST /start` | | `startupTimeout()` | 60s | stack and control API becoming reachable | | `eventTimeout()` | 12s | waiting for a provider event; exceed your worst-case detection latency | diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java index 7e7bf0dfbd..350cf83b47 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java @@ -101,6 +101,18 @@ public Map> additionalPorts() { return Collections.emptyMap(); } + /** + * Returns environment variables to pass to Compose, for a stack whose file reads them. + * + *

For a backend whose state has to be visible to the test JVM and not only inside the stack, + * where the host path is the test's to choose and the stack's to be told. + * + * @return environment for the Compose invocation, empty by default + */ + public Map composeEnvironment() { + return Collections.emptyMap(); + } + /** * Returns the backend configuration name passed to {@code POST /start?config=...}; distinct * from {@link ProviderTckHarness#configuration()}. @@ -174,6 +186,9 @@ private ComposeContainer startCompose() { } ComposeContainer stack = new ComposeContainer(composeFile); + for (Map.Entry variable : composeEnvironment().entrySet()) { + stack.withEnv(variable.getKey(), variable.getValue()); + } stack.withExposedService(backendService(), controlPort(), Wait.forListeningPort()); for (Integer port : backendPorts()) { stack.withExposedService(backendService(), port, Wait.forListeningPort());