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 0c07a65 commit 51a1330
Show file tree
Hide file tree
Showing 58 changed files with 636 additions and 69 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';

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
94 changes: 94 additions & 0 deletions frontend/src/app/DevFeatureFlagsBanner.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
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 as keyof DashboardCommonConfig, 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 51a1330

Please sign in to comment.