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.

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

refactor(ui): stop using /meta/config endpoint #3684

Merged
merged 2 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 6 additions & 12 deletions cmd/flipt/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,18 +324,12 @@
logger.Debug("local state directory exists", zap.String("path", cfg.Meta.StateDirectory))
}

info := info.Flipt{
Commit: commit,
BuildDate: date,
GoVersion: goVersion,
Version: version,
LatestVersion: releaseInfo.LatestVersion,
LatestVersionURL: releaseInfo.LatestVersionURL,
IsRelease: isRelease,
UpdateAvailable: releaseInfo.UpdateAvailable,
OS: goOS,
Arch: goArch,
}
info := info.New(
info.WithBuild(commit, date, goVersion, version, isRelease),
info.WithLatestRelease(releaseInfo),
info.WithOS(goOS, goArch),
info.WithConfig(cfg),
)

Check warning on line 332 in cmd/flipt/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/flipt/main.go#L327-L332

Added lines #L327 - L332 were not covered by tests

if cfg.Meta.TelemetryEnabled {
logger := logger.With(zap.String("component", "telemetry"))
Expand Down
10 changes: 10 additions & 0 deletions internal/config/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ type StorageConfig struct {
ReadOnly *bool `json:"readOnly,omitempty" mapstructure:"read_only,omitempty" yaml:"read_only,omitempty"`
}

func (c *StorageConfig) Info() map[string]string {
if c.Type == GitStorageType {
return map[string]string{
"repository": c.Git.Repository,
"ref": c.Git.Ref,
}
}
return nil
}

func (c *StorageConfig) setDefaults(v *viper.Viper) error {
switch v.GetString("storage.type") {
case string(LocalStorageType):
Expand Down
25 changes: 25 additions & 0 deletions internal/config/storage_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package config

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestStorageConfigInfo(t *testing.T) {
tests := []struct {
config StorageConfig
expected map[string]string
}{
{StorageConfig{Type: DatabaseStorageType}, nil},
{StorageConfig{Type: GitStorageType, Git: &StorageGitConfig{Repository: "repo1", Ref: "v1.0.0"}}, map[string]string{
"ref": "v1.0.0", "repository": "repo1",
}},
}

for _, tt := range tests {
t.Run(string(tt.config.Type), func(t *testing.T) {
assert.Equal(t, tt.expected, tt.config.Info())
})
}
}
89 changes: 79 additions & 10 deletions internal/info/flipt.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,88 @@ package info
import (
"encoding/json"
"net/http"

"go.flipt.io/flipt/internal/config"
"go.flipt.io/flipt/internal/release"
)

func New(opts ...Option) Flipt {
f := Flipt{}
for _, opt := range opts {
opt(&f)
}
return f
}

func WithBuild(commit, date, goVersion, version string, isRelease bool) Option {
return func(f *Flipt) {
f.Commit = commit
f.BuildDate = date
f.GoVersion = goVersion
f.IsRelease = isRelease
f.Version = version
}
}

func WithLatestRelease(releaseInfo release.Info) Option {
return func(f *Flipt) {
f.LatestVersion = releaseInfo.LatestVersion
f.LatestVersionURL = releaseInfo.LatestVersionURL
f.UpdateAvailable = releaseInfo.UpdateAvailable
}
}

func WithOS(os, arch string) Option {
return func(f *Flipt) {
f.OS = os
f.Arch = arch
}
}

func WithConfig(cfg *config.Config) Option {
return func(f *Flipt) {
f.Authentication = authentication{Required: cfg.Authentication.Required}
f.Storage = storage{Type: cfg.Storage.Type, Metadata: cfg.Storage.Info()}
f.Analytics = &analytics{Enabled: cfg.Analytics.Enabled()}
f.UI = &ui{Theme: cfg.UI.DefaultTheme, TopbarColor: cfg.UI.Topbar.Color}
}
}

type Option func(f *Flipt)

type authentication struct {
Required bool `json:"required"`
}

type analytics struct {
Enabled bool `json:"enabled,omitempty"`
}

type storage struct {
Type config.StorageType `json:"type"`
Metadata map[string]string `json:"metadata,omitempty"`
}

type ui struct {
Theme config.UITheme `json:"theme,omitempty"`
TopbarColor string `json:"topbarColor,omitempty"`
}

type Flipt struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have an opportunity to refactor this info API endpoint now I think?

maybe we should make the response it a bit more structured, something like:

{
  version: {
    current:
    latest: {
      version:
      URL:
    },
   build: {
    date: ,
    commit:
    goVersion:
    release:
   },
   authentication: {
    required: 
   },
   analytics: {
     enabled:
   },
   storage: {
    ...
  },
}

this would be an API response breaking change, but since the API is not documented for this endpoint maybe its ok?

wdyt?

Copy link
Collaborator Author

@erka erka Dec 5, 2024

Choose a reason for hiding this comment

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

@markphelps I’ve updated the new fields to match the desired format. However, I'm not ready to update old fields in this PR, as it would introduce too many changes at once.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds good to me

Version string `json:"version,omitempty"`
LatestVersion string `json:"latestVersion,omitempty"`
LatestVersionURL string `json:"latestVersionURL,omitempty"`
Commit string `json:"commit,omitempty"`
BuildDate string `json:"buildDate,omitempty"`
GoVersion string `json:"goVersion,omitempty"`
UpdateAvailable bool `json:"updateAvailable"`
IsRelease bool `json:"isRelease"`
OS string `json:"os,omitempty"`
Arch string `json:"arch,omitempty"`
Version string `json:"version,omitempty"`
LatestVersion string `json:"latestVersion,omitempty"`
LatestVersionURL string `json:"latestVersionURL,omitempty"`
Commit string `json:"commit,omitempty"`
BuildDate string `json:"buildDate,omitempty"`
GoVersion string `json:"goVersion,omitempty"`
UpdateAvailable bool `json:"updateAvailable"`
IsRelease bool `json:"isRelease"`
OS string `json:"os,omitempty"`
Arch string `json:"arch,omitempty"`
Authentication authentication `json:"authentication"`
Storage storage `json:"storage"`
Analytics *analytics `json:"analytics,omitempty"`
UI *ui `json:"ui,omitempty"`
}

func (f Flipt) IsDevelopment() bool {
Expand Down
44 changes: 44 additions & 0 deletions internal/info/flipt_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package info

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
"go.flipt.io/flipt/internal/config"
"go.flipt.io/flipt/internal/release"
)

func TestNew(t *testing.T) {
f := New(
WithOS("linux", "amd64"),
WithBuild("commit", "date", "goVersion", "version", true),
WithLatestRelease(release.Info{LatestVersion: "latestVersion", LatestVersionURL: "latestVersionURL", UpdateAvailable: true}),
WithConfig(config.Default()),
)

assert.Equal(t, "commit", f.Commit)
assert.Equal(t, "date", f.BuildDate)
assert.Equal(t, "goVersion", f.GoVersion)
assert.Equal(t, "version", f.Version)
assert.True(t, f.IsRelease)
assert.Equal(t, "latestVersion", f.LatestVersion)
assert.Equal(t, "latestVersionURL", f.LatestVersionURL)
assert.True(t, f.UpdateAvailable)
assert.Equal(t, "linux", f.OS)
assert.Equal(t, "amd64", f.Arch)
assert.False(t, f.Authentication.Required)
assert.False(t, f.Analytics.Enabled)
assert.Equal(t, config.DatabaseStorageType, f.Storage.Type)
}

func TestHttpHandler(t *testing.T) {
f := New()
f.Storage.Type = config.DatabaseStorageType
r := httptest.NewRequest("GET", "/info", nil)
w := httptest.NewRecorder()
f.ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, `{"updateAvailable":false,"isRelease":false,"authentication":{"required":false},"storage":{"type":"database"}}`, w.Body.String())
}
7 changes: 1 addition & 6 deletions ui/src/app/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ import Sidebar from '~/components/Sidebar';
import { useSession } from '~/data/hooks/session';
import { useAppDispatch } from '~/data/hooks/store';
import { LoadingStatus } from '~/types/Meta';
import {
fetchConfigAsync,
fetchInfoAsync,
selectConfig
} from './meta/metaSlice';
import { fetchInfoAsync, selectConfig } from './meta/metaSlice';
import {
currentNamespaceChanged,
selectCurrentNamespace,
Expand Down Expand Up @@ -62,7 +58,6 @@ function InnerLayout() {

useEffect(() => {
dispatch(fetchInfoAsync());
dispatch(fetchConfigAsync());
}, [dispatch]);

if (!session) {
Expand Down
36 changes: 16 additions & 20 deletions ui/src/app/meta/metaSlice.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/no-use-before-define */
import { createAsyncThunk, createSlice } from '@reduxjs/toolkit';
import { getConfig, getInfo } from '~/data/api';
import { getInfo } from '~/data/api';
import { IConfig, IInfo, LoadingStatus, StorageType } from '~/types/Meta';
import { Theme } from '~/types/Preferences';

Expand Down Expand Up @@ -40,23 +40,27 @@ export const metaSlice = createSlice({
reducers: {},
extraReducers(builder) {
builder
.addCase(fetchInfoAsync.fulfilled, (state, action) => {
state.info = action.payload;
})
.addCase(fetchConfigAsync.pending, (state, _action) => {
.addCase(fetchInfoAsync.pending, (state, _action) => {
state.config.status = LoadingStatus.LOADING;
})
.addCase(fetchConfigAsync.fulfilled, (state, action) => {
state.config = action.payload;
.addCase(fetchInfoAsync.fulfilled, (state, action) => {
state.info = action.payload;
state.config.status = LoadingStatus.SUCCEEDED;
state.config.analyticsEnabled =
action.payload.analytics.storage.clickhouse?.enabled ||
action.payload.analytics.storage.prometheus?.enabled;
if (action.payload.storage?.readOnly === undefined) {
action.payload.analytics?.enabled || false;
if (action.payload.storage !== undefined) {
state.config.storage.type = action.payload.storage?.type;
state.config.storage.git = action.payload.storage?.metadata;
state.config.storage.readOnly =
action.payload.storage?.type &&
action.payload.storage?.type !== StorageType.DATABASE;
action.payload.storage &&
action.payload.storage.type !== StorageType.DATABASE;
}
state.config.ui = {
defaultTheme: action.payload.ui?.Theme || Theme.SYSTEM,
topbar: {
color: action.payload.ui?.TopbarColor || ''
}
};
});
}
});
Expand All @@ -71,12 +75,4 @@ export const fetchInfoAsync = createAsyncThunk('meta/fetchInfo', async () => {
return response;
});

export const fetchConfigAsync = createAsyncThunk(
'meta/fetchConfig',
async () => {
const response = await getConfig();
return response;
}
);

export default metaSlice.reducer;
6 changes: 3 additions & 3 deletions ui/src/app/preferences/preferencesSlice.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* eslint-disable @typescript-eslint/no-use-before-define */
import { createSlice } from '@reduxjs/toolkit';
import { fetchConfigAsync } from '~/app/meta/metaSlice';
import { RootState } from '~/store';
import { Theme, Timezone } from '~/types/Preferences';
import { fetchInfoAsync } from '~/app/meta/metaSlice';

export const preferencesKey = 'preferences';

Expand All @@ -28,14 +28,14 @@ export const preferencesSlice = createSlice({
}
},
extraReducers(builder) {
builder.addCase(fetchConfigAsync.fulfilled, (state, action) => {
builder.addCase(fetchInfoAsync.fulfilled, (state, action) => {
const currentPreference = JSON.parse(
localStorage.getItem(preferencesKey) || '{}'
) as IPreferencesState;

// If there isn't currently a set theme, set to the default theme
if (!currentPreference.theme) {
state.theme = action.payload.ui.defaultTheme;
state.theme = action.payload.uiTheme;
}

if (!currentPreference.timezone) {
Expand Down
14 changes: 7 additions & 7 deletions ui/src/components/SessionProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createContext, useEffect, useMemo } from 'react';
import { getAuthSelf, getConfig, getInfo } from '~/data/api';
import { getAuthSelf, getInfo } from '~/data/api';
import { useLocalStorage } from '~/data/hooks/storage';
import { IAuthGithubInternal } from '~/types/auth/Github';
import { IAuthJWTInternal } from '~/types/auth/JWT';
Expand Down Expand Up @@ -27,16 +27,16 @@ export default function SessionProvider({
const [session, setSession, clearSession] = useLocalStorage('session', null);

useEffect(() => {
const clearSessionIfNecessary = async () => {
const config = await getConfig();
if (session && session.required !== config.authentication.required) {
const clearSessionIfNecessary = async (required: boolean) => {
if (session && session.required !== required) {
clearSession();
}
};

const loadSession = async () => {
let info: any = null;
try {
await getInfo();
info = await getInfo();
} catch (err) {
// if we can't get the info, we're not logged in
// or there was an error, either way, clear the session so we redirect
Expand All @@ -45,8 +45,8 @@ export default function SessionProvider({
return;
}

if (session) {
clearSessionIfNecessary();
if (session && info) {
clearSessionIfNecessary(info.authentication?.required);
if (session) {
return;
}
Expand Down
4 changes: 0 additions & 4 deletions ui/src/data/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,3 @@ async function getMeta(path: string) {
export async function getInfo() {
return getMeta('/info');
}

export async function getConfig() {
return getMeta('/config');
}
4 changes: 2 additions & 2 deletions ui/tests/flags.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ test.describe('Flags', () => {

test.describe('Flags - Read Only', () => {
test.beforeEach(async ({ page }) => {
await page.route(/\/meta\/config/, async (route) => {
await page.route(/\/meta\/info/, async (route) => {
const response = await route.fetch();
const json = await response.json();
json.storage = { type: 'git' };
json.storage.type = 'git';
// Fulfill using the original response, while patching the
// response body with our changes to mock git storage for read only mode
await route.fulfill({ response, json });
Expand Down
4 changes: 2 additions & 2 deletions ui/tests/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ test.describe('Root', () => {

test.describe('Root - Read Only', () => {
test.beforeEach(async ({ page }) => {
await page.route(/\/meta\/config/, async (route) => {
await page.route(/\/meta\/info/, async (route) => {
const response = await route.fetch();
const json = await response.json();
json.storage = { type: 'git' };
json.storage.type = 'git';
// Fulfill using the original response, while patching the
// response body with our changes to mock git storage for read only mode
await route.fulfill({ response, json });
Expand Down
Loading
Loading