Skip to content

Commit

Permalink
support feature flag overrides via query string
Browse files Browse the repository at this point in the history
  • Loading branch information
christianvogt committed Jun 21, 2024
1 parent 2cafeff commit 9cb95ae
Show file tree
Hide file tree
Showing 44 changed files with 596 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const updateClusterSettings = async (
notebookTolerationSettings,
modelServingPlatformEnabled,
} = request.body;
const dashConfig = getDashboardConfig();
const dashConfig = getDashboardConfig(request);
const isJupyterEnabled = checkJupyterEnabled();
try {
if (
Expand Down Expand Up @@ -124,10 +124,11 @@ export const updateClusterSettings = async (

export const getClusterSettings = async (
fastify: KubeFastifyInstance,
request: FastifyRequest,
): Promise<ClusterSettings | string> => {
const coreV1Api = fastify.kube.coreV1Api;
const namespace = fastify.kube.namespace;
const dashConfig = getDashboardConfig();
const dashConfig = getDashboardConfig(request);
const isJupyterEnabled = checkJupyterEnabled();
const clusterSettings: ClusterSettings = {
...DEFAULT_CLUSTER_SETTINGS,
Expand Down
2 changes: 1 addition & 1 deletion backend/src/routes/api/cluster-settings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default async (fastify: FastifyInstance): Promise<void> => {
fastify.get(
'/',
secureAdminRoute(fastify)(async (request: FastifyRequest, reply: FastifyReply) => {
return getClusterSettings(fastify)
return getClusterSettings(fastify, request)
.then((res) => {
return res;
})
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/storage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default async (fastify: FastifyInstance): Promise<void> => {
reply: FastifyReply,
) => {
try {
const dashConfig = getDashboardConfig();
const dashConfig = getDashboardConfig(request);
if (dashConfig?.spec.dashboardConfig.disableS3Endpoint !== false) {
reply.code(404).send('Not found');
return reply;
Expand Down Expand Up @@ -49,7 +49,7 @@ export default async (fastify: FastifyInstance): Promise<void> => {
reply: FastifyReply,
) => {
try {
const dashConfig = getDashboardConfig();
const dashConfig = getDashboardConfig(request);
if (dashConfig?.spec.dashboardConfig.disableS3Endpoint !== false) {
reply.code(404).send('Not found');
return reply;
Expand Down
29 changes: 27 additions & 2 deletions backend/src/utils/resourceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { getIsAppEnabled, getRouteForApplication, getRouteForClusterId } from '.
import { createCustomError } from './requestUtils';
import { getDetectedAccelerators } from '../routes/api/accelerators/acceleratorUtils';
import { RecursivePartial } from '../typeHelpers';
import { FastifyRequest } from 'fastify';

const dashboardConfigMapName = 'odh-dashboard-config';
const consoleLinksGroup = 'console.openshift.io';
Expand Down Expand Up @@ -548,8 +549,32 @@ export const initializeWatchedResources = (fastify: KubeFastifyInstance): void =
consoleLinksWatcher = new ResourceWatcher<ConsoleLinkKind>(fastify, fetchConsoleLinks);
};

export const getDashboardConfig = (): DashboardConfig => {
return dashboardConfigWatcher.getResources()?.[0];
const FEATURE_FLAGS_HEADER = 'x-odh-feature-flags';

// if inspecting feature flags, provide the request to ensure overridden feature flags are considered
export const getDashboardConfig = (request?: FastifyRequest): DashboardConfig => {
const dashboardConfig = dashboardConfigWatcher.getResources()?.[0];
if (request) {
const flagsHeader = request.headers[FEATURE_FLAGS_HEADER];
if (typeof flagsHeader === 'string') {
try {
const featureFlags = JSON.parse(flagsHeader);
return {
...dashboardConfig,
spec: {
...dashboardConfig.spec,
dashboardConfig: {
...dashboardConfig.spec.dashboardConfig,
...featureFlags,
},
},
};
} catch {
// ignore
}
}
}
return dashboardConfig;
};

export const updateDashboardConfig = (): Promise<void> => {
Expand Down
18 changes: 14 additions & 4 deletions frontend/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,25 @@
"no-restricted-imports": [
"error",
{
"paths": [
{
"name": "^axios$",
"importNames": ["default"],
"message": "Import from `~/utilities/axios` instead."
}
],
"patterns": [
{
"group": ["~/api/**"],
"message": "Read from '~/api' instead."
"message": "Import from '~/api' instead."
},
{
"group": ["~/components/table/**", "!~/components/table/useTableColumnSort"],
"message": "Read from '~/components/table' instead."
"message": "Import from '~/components/table' instead."
},
{
"group": ["~/concepts/area/**"],
"message": "Read from '~/concepts/area' instead."
"message": "Import from '~/concepts/area' instead."
},
{
"group": ["~/components/table/useTableColumnSort"],
Expand Down Expand Up @@ -286,7 +293,10 @@
],
"overrides": [
{
"files": ["./src/__tests__/cypress/cypress/pages/*.ts", "./src/__tests__/cypress/cypress/tests/e2e/*.ts"],
"files": [
"./src/__tests__/cypress/cypress/pages/*.ts",
"./src/__tests__/cypress/cypress/tests/e2e/*.ts"
],
"rules": {
"no-restricted-syntax": [
"error",
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/api/k8s/projects.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import axios from 'axios';
import {
k8sCreateResource,
k8sDeleteResource,
k8sListResource,
K8sResourceCommon,
k8sUpdateResource,
} from '@openshift/dynamic-plugin-sdk-utils';
import axios from '~/utilities/axios';
import { CustomWatchK8sResult } from '~/types';
import { K8sAPIOptions, ProjectKind } from '~/k8sTypes';
import { ProjectModel, ProjectRequestModel } from '~/api/models';
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/api/prometheus/usePrometheusQuery.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import axios from 'axios';
import axios from '~/utilities/axios';
import { PrometheusQueryResponse } from '~/types';
import useFetchState, { FetchOptions, FetchState, NotReadyError } from '~/utilities/useFetchState';

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/api/prometheus/usePrometheusQueryRange.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import axios from 'axios';
import axios from '~/utilities/axios';

import useFetchState, {
FetchOptions,
Expand Down
10 changes: 9 additions & 1 deletion frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,24 @@ import { useApplicationSettings } from './useApplicationSettings';
import TelemetrySetup from './TelemetrySetup';
import { logout } from './appUtils';
import QuickStarts from './QuickStarts';
import DevFeatureFlagsBanner from './DevFeatureFlagsBanner';

import './App.scss';
import useDevFeatureFlags from '~/app/useDevFeatureFlags';

Check failure on line 35 in frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / Tests (18.x)

`~/app/useDevFeatureFlags` import should occur before import of `./Header`

const App: React.FC = () => {
const [notificationsOpen, setNotificationsOpen] = React.useState(false);
const { username, userError, isAllowed } = useUser();

const buildStatuses = useWatchBuildStatus();
const {
dashboardConfig,
dashboardConfig: dashboardConfigFromServer,
loaded: configLoaded,
loadError: fetchConfigError,
} = useApplicationSettings();

const { dashboardConfig, ...devFeatureFlags } = useDevFeatureFlags(dashboardConfigFromServer);

const [storageClasses] = useStorageClasses();

useDetectUser();
Expand Down Expand Up @@ -115,6 +119,10 @@ const App: React.FC = () => {
data-testid={DASHBOARD_MAIN_CONTAINER_ID}
>
<ErrorBoundary>
<DevFeatureFlagsBanner
dashboardConfig={dashboardConfig.spec.dashboardConfig}
{...devFeatureFlags}
/>
<ProjectsContextProvider>
<QuickStarts>
<AppRoutes />
Expand Down
92 changes: 92 additions & 0 deletions frontend/src/app/DevFeatureFlagsBanner.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { Banner, Button, Checkbox, Grid, GridItem, Modal } from '@patternfly/react-core';
import * as React from 'react';
import { allFeatureFlags } from '~/concepts/areas/const';
import { isFeatureFlag } from '~/concepts/areas/utils';
import { DashboardCommonConfig } from '~/k8sTypes';
import { DevFeatureFlags } from '~/types';

type Props = { dashboardConfig: Partial<DashboardCommonConfig> } & DevFeatureFlags;

const DevdevFeatureFlagsBanner: React.FC<Props> = ({
dashboardConfig,
devFeatureFlags,
setDevFeatureFlag,
resetDevFeatureFlags,
}) => {
const [isModalOpen, setModalOpen] = React.useState(false);
if (!devFeatureFlags) {
return null;
}
const renderDevFeatureFlags = () => (
<Grid hasGutter span={6} md={3}>
{allFeatureFlags
.filter(isFeatureFlag)
.toSorted()
.map((key) => {
const value = devFeatureFlags[key] ?? dashboardConfig[key];
return (
<React.Fragment key={key}>
<GridItem>
<Checkbox
id={key}
data-testid={`${key}-checkbox`}
label={key}
isChecked={value ?? null}
onChange={(_, checked) => setDevFeatureFlag(key, checked)}
/>
</GridItem>
<GridItem data-testid={`${key}-value`}>{`${value ?? ''}${
key in devFeatureFlags ? ' (overridden)' : ''
}`}</GridItem>
</React.Fragment>
);
})}
</Grid>
);
return (
<>
<Banner variant="blue">
Feature flags are{' '}
<Button
data-testid="override-feature-flags-button"
variant="link"
isInline
onClick={() => setModalOpen(true)}
>
overridden
</Button>{' '}
in the current session.{' '}
<Button
data-testid="reset-feature-flags-button"
variant="link"
isInline
onClick={resetDevFeatureFlags}
>
Click here
</Button>{' '}
to reset back to default.
</Banner>
<Modal
data-testid="dev-feature-flags-modal"
variant="large"
title="Feature flags"
isOpen={isModalOpen}
onClose={() => setModalOpen(false)}
actions={[
<Button
data-testid="reset-feature-flags-modal-button"
key="confirm"
variant="link"
onClick={() => resetDevFeatureFlags()}
>
Reset to defaults
</Button>,
]}
>
{renderDevFeatureFlags()}
</Modal>
</>
);
};

export default DevdevFeatureFlagsBanner;
80 changes: 80 additions & 0 deletions frontend/src/app/__tests__/DevFeatureFlagsBanner.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import * as React from 'react';
import { act, render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';
import DevFeatureFlagsBanner from '~/app/DevFeatureFlagsBanner';

describe('DevFeatureFlagsBanner', () => {
it('should render not render if no feature flags are overridden', () => {
const result = render(
<DevFeatureFlagsBanner
dashboardConfig={{}}
setDevFeatureFlag={() => undefined}
resetDevFeatureFlags={() => undefined}
devFeatureFlags={null}
/>,
);
expect(result.container).toBeEmptyDOMElement();
});

it('should render banner and open modal', () => {
const resetFn = jest.fn();
const result = render(
<DevFeatureFlagsBanner
dashboardConfig={{}}
setDevFeatureFlag={() => undefined}
resetDevFeatureFlags={resetFn}
devFeatureFlags={{}}
/>,
);
expect(result.container).not.toBeEmptyDOMElement();
act(() => result.getByTestId('override-feature-flags-button').click());
result.getByTestId('dev-feature-flags-modal');

act(() => result.getByTestId('reset-feature-flags-button').click());
expect(resetFn).toHaveBeenCalled();
});

it('should render and set feature flags', () => {
const setFeatureFlagFn = jest.fn();
const resetFn = jest.fn();
const result = render(
<DevFeatureFlagsBanner
dashboardConfig={{
disableAcceleratorProfiles: false,
}}
setDevFeatureFlag={setFeatureFlagFn}
resetDevFeatureFlags={resetFn}
devFeatureFlags={{
disableHome: true,
}}
/>,
);
expect(result.container).not.toBeEmptyDOMElement();
act(() => result.getByTestId('override-feature-flags-button').click());
screen.getByTestId('dev-feature-flags-modal');

act(() => result.getByTestId('reset-feature-flags-button').click());
expect(resetFn).toHaveBeenCalled();

expect(result.getByTestId('disableHome-checkbox')).toBeChecked();
expect(result.getByTestId('disableAcceleratorProfiles-checkbox')).not.toBeChecked();
expect(result.getByTestId('enablement-checkbox')).toBePartiallyChecked();

expect(result.getByTestId('disableHome-value').textContent).toBe('true (overridden)');
expect(result.getByTestId('disableAcceleratorProfiles-value').textContent).toBe('false');
expect(result.getByTestId('enablement-value').textContent).toBe('');

act(() => {
result.getByTestId('disableHome-checkbox').click();
result.getByTestId('disableAcceleratorProfiles-checkbox').click();
result.getByTestId('enablement-checkbox').click();
});

expect(setFeatureFlagFn).toHaveBeenCalledTimes(3);
expect(setFeatureFlagFn.mock.calls).toEqual([
['disableHome', false],
['disableAcceleratorProfiles', true],
['enablement', true],
]);
});
});
Loading

0 comments on commit 9cb95ae

Please sign in to comment.