Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
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