Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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 @@ -65,8 +65,9 @@ export function getFeatureFlagsInLatestCompositionByFederatedGraph(
}

// Get feature flag IDs from the latest valid composition
const ffsInLatestValidComposition = await featureFlagRepo.getFeatureFlagSchemaVersionsByBaseSchemaVersion({
const ffsInLatestValidComposition = await featureFlagRepo.getFeatureFlagSchemaVersionsInLatestComposition({
baseSchemaVersionId: federatedGraph.schemaVersionId,
federatedGraphId: federatedGraph.id,
});

const featureFlags: FeatureFlagDTO[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,22 @@ export function getCompositionDetails(
};
}

if (composition.targetId) {
const graph = await fedRepo.byTargetId(composition.targetId);
if (graph && !authContext.rbac.hasFederatedGraphReadAccess(graph)) {
throw new UnauthorizedError();
}
// A composition is always tied to a federated graph (its target is only nulled once the graph is deleted), so an
// unresolvable graph means the composition is orphaned — treat it as not found rather than serving it without an
// authorization check.
const graph = composition.targetId ? await fedRepo.byTargetId(composition.targetId) : undefined;
if (!graph) {
return {
response: {
code: EnumStatusCode.ERR_NOT_FOUND,
details: `Federated graph for composition '${req.compositionId}' not found`,
},
compositionSubgraphs: [],
featureFlagCompositions: [],
};
}
if (!authContext.rbac.hasFederatedGraphReadAccess(graph)) {
throw new UnauthorizedError();
}

const compositionSubgraphs = await compositionRepo.getCompositionSubgraphs({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ export function getFederatedGraphById(
const featureFlagsInLatestValidComposition: FeatureFlagDTO[] = [];

if (federatedGraph.schemaVersionId) {
const ffsInLatestValidComposition = await featureFlagRepo.getFeatureFlagSchemaVersionsByBaseSchemaVersion({
const ffsInLatestValidComposition = await featureFlagRepo.getFeatureFlagSchemaVersionsInLatestComposition({
baseSchemaVersionId: federatedGraph.schemaVersionId,
federatedGraphId: federatedGraph.id,
});
if (ffsInLatestValidComposition) {
for (const ff of ffsInLatestValidComposition) {
Expand Down
24 changes: 17 additions & 7 deletions controlplane/src/core/repositories/FeatureFlagRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1483,16 +1483,19 @@ export class FeatureFlagRepository {
return featureFlagCompositions;
}

// return all the feature flag schema versions associated with the base schema version
// input: base schema version id
public async getFeatureFlagSchemaVersionsByBaseSchemaVersion({
public async getFeatureFlagSchemaVersionsInLatestComposition({
baseSchemaVersionId,
federatedGraphId,
}: {
baseSchemaVersionId: string;
federatedGraphId: string;
}) {
// A feature flag can have multiple composed schema versions against the same base schema version
// (e.g. recomposing the feature flag recomposes it against the unchanged base, so rows accumulate).
// Deduplicate by feature flag, keeping the latest composed version, so callers get one entry per flag.
const orgRepo = new OrganizationRepository(this.logger, this.db);
const splitConfigFeature = await orgRepo.getFeature({
organizationId: this.organizationId,
featureId: 'split-config-loading',
});

const ffSchemaVersions = await this.db
.selectDistinctOn([federatedGraphsToFeatureFlagSchemaVersions.featureFlagId], {
id: federatedGraphsToFeatureFlagSchemaVersions.composedSchemaVersionId,
Expand All @@ -1503,7 +1506,14 @@ export class FeatureFlagRepository {
schemaVersion,
eq(schemaVersion.id, federatedGraphsToFeatureFlagSchemaVersions.composedSchemaVersionId),
)
.where(eq(federatedGraphsToFeatureFlagSchemaVersions.baseCompositionSchemaVersionId, baseSchemaVersionId))
.where(
splitConfigFeature?.enabled
? and(
isNull(federatedGraphsToFeatureFlagSchemaVersions.baseCompositionSchemaVersionId),
eq(federatedGraphsToFeatureFlagSchemaVersions.federatedGraphId, federatedGraphId),
)
: eq(federatedGraphsToFeatureFlagSchemaVersions.baseCompositionSchemaVersionId, baseSchemaVersionId),
)
.orderBy(federatedGraphsToFeatureFlagSchemaVersions.featureFlagId, desc(schemaVersion.createdAt))
.execute();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('GetFeatureFlagsInLatestCompositionByFederatedGraph', () => {
expect(resp.featureFlags.some((f) => f.name === flagName)).toBe(true);
});

test('that feature flag compositions are decoupled when split config loading is enabled', async (testContext) => {
test('that feature flags in the latest composition are returned when split config loading is enabled', async (testContext) => {
const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] });
testContext.onTestFinished(() => server.close());

Expand Down Expand Up @@ -118,7 +118,8 @@ describe('GetFeatureFlagsInLatestCompositionByFederatedGraph', () => {
});

expect(resp.response?.code).toBe(EnumStatusCode.OK);
expect(resp.featureFlags).toHaveLength(0);
expect(resp.featureFlags).toHaveLength(1);
expect(resp.featureFlags.some((f) => f.name === flagName)).toBe(true);

// Create a second, enabled feature flag
const secondFlagName = genID('flag');
Expand All @@ -129,7 +130,9 @@ describe('GetFeatureFlagsInLatestCompositionByFederatedGraph', () => {
namespace,
});
expect(withSecondFlag.response?.code).toBe(EnumStatusCode.OK);
expect(withSecondFlag.featureFlags).toHaveLength(0);
expect(withSecondFlag.featureFlags).toHaveLength(2);
expect(withSecondFlag.featureFlags.some((f) => f.name === flagName)).toBe(true);
expect(withSecondFlag.featureFlags.some((f) => f.name === secondFlagName)).toBe(true);

// Only the base graph composition should show up when excluding feature flag compositions
let compositionsResp = await client.getCompositions({
Expand Down Expand Up @@ -194,6 +197,151 @@ describe('GetFeatureFlagsInLatestCompositionByFederatedGraph', () => {
expect(resp.featureFlags).toHaveLength(0);
});

test('that a feature flag is still returned when its latest composition failed', async (testContext) => {
const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] });
testContext.onTestFinished(() => server.close());

const namespace = genID('namespace').toLowerCase();
const labels = [genUniqueLabel()];
const federatedGraphName = genID('fedGraph');

await createNamespace(client, namespace);

await createAndPublishSubgraph(
client,
'users',
namespace,
fs.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/users.graphql')).toString(),
labels,
DEFAULT_SUBGRAPH_URL_ONE,
);

await createAndPublishSubgraph(
client,
'products-standalone',
namespace,
fs.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/products-standalone.graphql')).toString(),
labels,
'http://localhost:4002',
);

// A feature subgraph for `users` that mirrors the base schema plus one extra field, so it composes on its own
await createThenPublishFeatureSubgraph(
client,
'users-ff',
'users',
namespace,
`
type User @key(fields: "id") {
id: ID!
name: String!
email: String!
isPremium: Boolean! @tag(name: "exclude")
nickname: String!
}

type Query {
user(id: ID!): User
users: [User!]!
}
`,
labels,
'http://localhost:4101',
);

await createThenPublishFeatureSubgraph(
client,
'products-standalone-feature',
'products-standalone',
namespace,
fs
.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/products-standalone-feature.graphql'))
.toString(),
labels,
'http://localhost:4102',
);

const federatedGraphLabels = labels.map(({ key, value }) => `${key}=${value}`);
await createFederatedGraph(client, federatedGraphName, namespace, federatedGraphLabels, DEFAULT_ROUTER_URL);

// Two enabled flags, each built on its own feature subgraph, both composing successfully
const successfulFlagName = genID('flag');
await createFeatureFlag(client, successfulFlagName, labels, ['products-standalone-feature'], namespace, true);

const failingFlagName = genID('flag');
await createFeatureFlag(client, failingFlagName, labels, ['users-ff'], namespace, true);

let resp = await client.getFeatureFlagsInLatestCompositionByFederatedGraph({
federatedGraphName,
namespace,
});

expect(resp.response?.code).toBe(EnumStatusCode.OK);
expect(resp.featureFlags).toHaveLength(2);

/**
* Break the composition of only the second flag by declaring `Product.details` with a type that conflicts with the
* `products-standalone` subgraph (`String!` there, `Int!` here). This is published to a feature subgraph, so the
* base composition — and therefore the graph's base schema version — is untouched, and the other flag still
* composes.
*/
const publishResp = await client.publishFederatedSubgraph({
name: 'users-ff',
namespace,
schema: `
type User @key(fields: "id") {
id: ID!
name: String!
email: String!
isPremium: Boolean! @tag(name: "exclude")
}

type Product @key(fields: "upc sku") {
upc: Int!
sku: String!
details: Int!
}

type Query {
user(id: ID!): User
users: [User!]!
}
`,
});
expect(publishResp.response?.code).toBe(EnumStatusCode.ERR_SUBGRAPH_COMPOSITION_FAILED);
expect(publishResp.compositionErrors.length).toBeGreaterThan(0);

// Confirm the failure landed as a non-composable composition for the failing flag only
const compositionsResp = await client.getCompositions({
fedGraphName: federatedGraphName,
namespace,
startDate: formatISO(subDays(new Date(), 1)),
endDate: formatISO(addMinutes(new Date(), 1)),
});
expect(compositionsResp.response?.code).toBe(EnumStatusCode.OK);
expect(compositionsResp.compositions.some((c) => c.featureFlagName === failingFlagName && !c.isComposable)).toBe(
true,
);
expect(compositionsResp.compositions.some((c) => c.featureFlagName === successfulFlagName && !c.isComposable)).toBe(
false,
);

/**
* Both flags are still in the latest composition. A failed composition does not remove the flag: its router config
* is not replaced, so the last valid composition keeps being served, and the flag remains attached and enabled.
*/
resp = await client.getFeatureFlagsInLatestCompositionByFederatedGraph({
federatedGraphName,
namespace,
});

expect(resp.response?.code).toBe(EnumStatusCode.OK);
expect(resp.featureFlags).toHaveLength(2);
expect(resp.featureFlags.map((f) => f.name).sort()).toStrictEqual(
[successfulFlagName, failingFlagName].sort((a, b) => a.localeCompare(b)),
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});

test('Should return ERR_NOT_FOUND for non-existent federated graph', async (testContext) => {
const { client, server } = await SetupTest({ dbname });
testContext.onTestFinished(() => server.close());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import fs from 'node:fs';
import { join } from 'node:path';
import { EnumStatusCode } from '@wundergraph/cosmo-connect/dist/common/common_pb';
import { afterAll, beforeAll, describe, expect, onTestFinished, test } from 'vitest';
import {
Expand All @@ -6,8 +8,20 @@ import {
createTestGroup,
createTestRBACEvaluator,
genID,
genUniqueLabel,
} from '../../src/core/test-util.js';
import { createFederatedGraph, createThenPublishSubgraph, DEFAULT_NAMESPACE, SetupTest } from '../test-util.js';
import {
createAndPublishSubgraph,
createFeatureFlag,
createFederatedGraph,
createNamespace,
createThenPublishFeatureSubgraph,
createThenPublishSubgraph,
DEFAULT_NAMESPACE,
DEFAULT_ROUTER_URL,
DEFAULT_SUBGRAPH_URL_ONE,
SetupTest,
} from '../test-util.js';

let dbname = '';

Expand Down Expand Up @@ -177,4 +191,57 @@ describe('GetFederatedGraphById', () => {
expect(response.response?.code).toBe(EnumStatusCode.ERROR_NOT_AUTHORIZED);
},
);

test('Should return the feature flags in the latest composition when split config loading is enabled', async (testContext) => {
const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] });
testContext.onTestFinished(() => server.close());

const namespace = genID('namespace').toLowerCase();
const labels = [genUniqueLabel()];
const federatedGraphName = genID('fedGraph');

await createNamespace(client, namespace);

await createAndPublishSubgraph(
client,
'users',
namespace,
fs.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/users.graphql')).toString(),
labels,
DEFAULT_SUBGRAPH_URL_ONE,
);

await createThenPublishFeatureSubgraph(
client,
'users-feature',
'users',
namespace,
fs.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/users-feature.graphql')).toString(),
labels,
'http://localhost:4101',
);

const federatedGraphLabels = labels.map(({ key, value }) => `${key}=${value}`);
await createFederatedGraph(client, federatedGraphName, namespace, federatedGraphLabels, DEFAULT_ROUTER_URL);

const flagName = genID('flag');
await createFeatureFlag(client, flagName, labels, ['users-feature'], namespace, true);

const graphByName = await client.getFederatedGraphByName({
name: federatedGraphName,
namespace,
});
expect(graphByName.response?.code).toBe(EnumStatusCode.OK);

// Under split config the feature flag composition rows have a null base linkage; getFederatedGraphById must still
// resolve them via the federated graph so the flag shows up in the latest composition (COSMO-315).
const response = await client.getFederatedGraphById({
id: graphByName.graph!.id,
includeMetrics: false,
});

expect(response.response?.code).toBe(EnumStatusCode.OK);
expect(response.featureFlagsInLatestValidComposition).toHaveLength(1);
expect(response.featureFlagsInLatestValidComposition.some((f) => f.name === flagName)).toBe(true);
});
});
Loading