Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SchemaEditor behind a feature flag #5564

Merged
merged 4 commits into from
Oct 26, 2022
Merged

SchemaEditor behind a feature flag #5564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4011bdd
Fix typo. Add missing type in test
antgamdia Oct 24, 2022
26eb1f6
Use the `schemaEditor` feature flag
antgamdia Oct 25, 2022
8d4f5da
Add `schemaEditor` as a feature flag in the chart
antgamdia Oct 25, 2022
018d2a7
Fix linter issue
antgamdia Oct 25, 2022
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
2 changes: 1 addition & 1 deletion chart/kubeapps/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ maintainers:
name: kubeapps
sources:
- https://github.com/vmware-tanzu/kubeapps
version: 11.0.2-dev1
version: 11.1.0-dev0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see who gets there first (you and Rafa are both updating in your PRs :) ).

19 changes: 13 additions & 6 deletions chart/kubeapps/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,12 +421,19 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith

### Other Parameters

| Name | Description | Value |
| ------------------------- | ----------------------------------------------------------------------------- | ------- |
| `allowNamespaceDiscovery` | Allow users to discover available namespaces (only the ones they have access) | `true` |
| `clusters` | List of clusters that Kubeapps can target for deployments | `[]` |
| `featureFlags.operators` | Enable ingress record generation for Kubeapps | `false` |
| `rbac.create` | Specifies whether RBAC resources should be created | `true` |
| Name | Description | Value |
| ------------------------- | ----------------------------------------------------------------------------- | ------ |
| `allowNamespaceDiscovery` | Allow users to discover available namespaces (only the ones they have access) | `true` |
| `clusters` | List of clusters that Kubeapps can target for deployments | `[]` |
| `rbac.create` | Specifies whether RBAC resources should be created | `true` |


### Feature flags

| Name | Description | Value |
| --------------------------- | ---------------------------------------------------------- | ------- |
| `featureFlags.operators` | Enable support for Operators in Kubeapps | `false` |
| `featureFlags.schemaEditor` | Enable a visual editor for customizing the package schemas | `false` |


### Database Parameters
Expand Down
18 changes: 11 additions & 7 deletions chart/kubeapps/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,20 +1417,24 @@ clusters:
- name: default
domain: cluster.local

## @skip featureFlags Feature flags (used to switch on development features)
##
featureFlags:
## @param featureFlags.operators Enable ingress record generation for Kubeapps
##
operators: false

## RBAC configuration
##
rbac:
## @param rbac.create Specifies whether RBAC resources should be created
##
create: true

## @section Feature flags

## Used enable some opt-in development features
##
featureFlags:
## @param featureFlags.operators Enable support for Operators in Kubeapps
operators: false
## @param featureFlags.schemaEditor Enable a visual editor for customizing the package schemas
##
schemaEditor: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine either way, just wondering if there may be other options for the schemaEditor in the future. If so, we may want to give it structure (schemaEditor.enabled: true so other options are possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, good point. I don't know, perhaps yes, there might be more options, but this would mean this feature is no longer "in development", but is something more "stable".
I'd rather keep it under a feature flag for now, so that we can perform backward incompatible changes if needed in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry - I wasn't proposing moving it from featureFlags, but just giving it structure rather than a bool (ie. the full path would be featureFlags.schemaEditor.enabled = true).

But fine either way, it's in development anyway :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, got it, got it. I didn't get your point :P Then, yes, it could be a good idea; I can perform the changes in the main #5530 PR


## @section Database Parameters

## PostgreSQL chart configuration
Expand Down
3 changes: 2 additions & 1 deletion dashboard/public/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"oauthLogoutURI": "/oauth2/sign_out",
"clusters": ["default"],
"featureFlags": {
"operators": false
"operators": false,
"schemaEditor": false
},
"theme": "light",
"remoteComponentsUrl": "",
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/actions/config.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const testConfig = {
oauthLogoutURI: "",
authProxySkipLoginPage: false,
clusters: [],
featureFlags: { operators: false },
featureFlags: { operators: false, schemaEditor: false },
theme: SupportedThemes.light,
remoteComponentsUrl: "",
customAppViews: [],
Expand Down
6 changes: 3 additions & 3 deletions dashboard/src/components/AppList/AppList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ afterEach(() => {
context("when changing props", () => {
it("should fetch apps in the new namespace", async () => {
const state = deepClone(initialState) as IStoreState;
state.config.featureFlags = { operators: true };
state.config.featureFlags = { ...initialState.config.featureFlags, operators: true };
const store = getStore(state);
const fetchInstalledPackages = jest.fn();
const getCustomResources = jest.fn();
Expand All @@ -71,7 +71,7 @@ context("when changing props", () => {

it("should not fetch resources in the new namespace when operators is deactivated", async () => {
const state = deepClone(initialState) as IStoreState;
state.config.featureFlags = { operators: false };
state.config.featureFlags = { ...initialState.config.featureFlags, operators: false };
const store = getStore(state);
const fetchInstalledPackages = jest.fn();
const getCustomResources = jest.fn();
Expand Down Expand Up @@ -110,7 +110,7 @@ context("when changing props", () => {

it("should fetch apps in all namespaces", async () => {
const state = deepClone(initialState) as IStoreState;
state.config.featureFlags = { operators: true };
state.config.featureFlags = { ...initialState.config.featureFlags, operators: true };
const store = getStore(state);
const fetchInstalledPackages = jest.fn();
const getCustomResources = jest.fn();
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/AppView/AppControls/DeleteButton/DeleteButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const defaultProps = {
};

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.installedpackages = {
...actions.installedpackages,
Expand All @@ -38,7 +38,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
spyOnUseDispatch.mockRestore();
});

Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/AppView/AppControls/RollbackButton/RollbackButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const defaultProps = {
};

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.installedpackages = {
...actions.installedpackages,
Expand All @@ -41,7 +41,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
spyOnUseDispatch.mockRestore();
});

Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/AppView/AppControls/UpgradeButton/UpgradeButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const defaultProps = {
};

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.installedpackages = {
...actions.installedpackages,
Expand All @@ -34,7 +34,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
spyOnUseDispatch.mockRestore();
});

Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/components/AppView/CustomAppView/CustomAppView.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IAppViewResourceRefs } from "../AppView";

const defaultState = {
config: { remoteComponentsUrl: "" },
};
} as IStoreState;

const defaultProps = {
app: {
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Catalog/Catalog.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ it("retrieves csvs in the namespace if operators enabled", () => {
const getCSVs = jest.fn();
actions.operators.getCSVs = getCSVs;
const state = deepClone(populatedState) as IStoreState;
state.config.featureFlags = { operators: true };
state.config.featureFlags = { ...initialState.config.featureFlags, operators: true };

mountWrapper(
getStore(state),
Expand All @@ -185,7 +185,7 @@ it("not retrieveing csvs in the namespace if operators deactivated", () => {
const getCSVs = jest.fn();
actions.operators.getCSVs = getCSVs;
const state = deepClone(populatedState) as IStoreState;
state.config.featureFlags = { operators: false };
state.config.featureFlags = { ...initialState.config.featureFlags, operators: false };

mountWrapper(
getStore(state),
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Config/PkgRepoList/PkgRepoButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jest.mock("./PkgRepoForm", () => {
});

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.repos = {
...actions.repos,
Expand All @@ -32,7 +32,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
spyOnUseDispatch.mockRestore();
});

Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Config/PkgRepoList/PkgRepoControl.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { PkgRepoAddButton } from "./PkgRepoButton";
import { IPkgRepoListItemProps, PkgRepoControl } from "./PkgRepoControl";

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.repos = {
...actions.repos,
Expand All @@ -23,7 +23,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
spyOnUseDispatch.mockRestore();
});

Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Config/PkgRepoList/PkgRepoForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const pkgRepoFormData = {
} as IPkgRepoFormData;

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.repos = {
...actions.repos,
Expand All @@ -99,7 +99,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
spyOnUseDispatch.mockRestore();
});

Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Config/PkgRepoList/PkgRepoList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const {
const namespace = clusters[currentCluster].currentNamespace;

let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.kube };
const kubeActions = { ...actions.kube };
beforeEach(() => {
actions.repos = {
...actions.repos,
Expand All @@ -43,7 +43,7 @@ beforeEach(() => {
});

afterEach(() => {
actions.kube = { ...kubeaActions };
actions.kube = { ...kubeActions };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup.

spyOnUseDispatch.mockRestore();
});

Expand Down
2 changes: 1 addition & 1 deletion ...mentFormBody/BasicDeploymentForm/TabularSchemaEditorTable/Params/CustomFormParam.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const defaultProps = {

const defaultState = {
config: { remoteComponentsUrl: "" },
};
} as IStoreState;

// Ensure remote-component doesn't trigger external requests during this test.
const mockOpen = jest.fn();
Expand Down
Loading