Skip to content

Commit 81628fd

Browse files
committed
Fix issues about visible fields in side panel
Previously, visible fields were determined out of several criteria including: - fields with calculated values were filtered out - record with null value was filtered out (as opposed to undefined) Also, "Common" columns was determined by having a calculated value (ie. non-common columns were prohibited to use calculated) This must change since columns can now have calculated and still must be visible (e.g. SrcK8S_Name is a kubeObject calculated value) - So, Common now is when calculated in the form of "[a,b]" - Visible columns for side panel is now based on being tied to fields
1 parent d867a10 commit 81628fd

File tree

8 files changed

+77
-56
lines changed

8 files changed

+77
-56
lines changed

web/src/components/__tests-data__/flows.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ export const FlowsSample: Record[] = [
3737
_RecordType: 'flowLog',
3838
FlowDirection: FlowDirection.Egress,
3939
SrcK8S_Namespace: 'default',
40-
DstK8S_Namespace: 'default'
40+
DstK8S_Namespace: 'default',
41+
SrcK8S_Type: 'Pod',
42+
DstK8S_Type: 'Pod'
4143
},
4244
key: 1,
4345
fields: {

web/src/components/drawer/record/__tests__/record-panel.spec.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@ describe('<RecordPanel />', () => {
2727
expect(wrapper.find(RecordPanel)).toBeTruthy();
2828
expect(wrapper.find('#record-panel-test')).toHaveLength(1);
2929
// all columns with data + JSON field
30-
// sample contains 18 fields
30+
// sample contains 20 fields
3131
// JSON tab represent 1 extra field
32-
expect(wrapper.find('.record-field-container')).toHaveLength(18 + 1);
32+
expect(wrapper.find('.record-field-container')).toHaveLength(20 + 1);
3333

3434
// same with 4 valid fields + json
3535
wrapper.setProps({ record: UnknownFlow });
3636
expect(wrapper.find('.record-field-container')).toHaveLength(4 + 1);
3737
});
38+
3839
it('should close on click', async () => {
3940
const wrapper = shallow(<RecordPanel {...mocks} />);
4041
const closeButton = wrapper.find(DrawerCloseButton);

web/src/components/drawer/record/record-panel.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ export const RecordPanel: React.FC<RecordDrawerProps> = ({
8181
const getVisibleColumns = React.useCallback(() => {
8282
const forbiddenColumns = [ColumnsId.ifdirs, ColumnsId.interfaces];
8383
return columns.filter((c: Column) => {
84-
if (!c.value) {
84+
if (!c.fieldValue) {
8585
return false;
8686
}
87-
const value = c.value(record);
87+
const value = c.fieldValue(record);
8888
return !forbiddenColumns.includes(c.id) && value !== '' && !Number.isNaN(value);
8989
});
9090
}, [columns, record]);

web/src/components/netflow-traffic.tsx

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,13 @@ export const NetflowTraffic: React.FC<NetflowTrafficProps> = ({
201201
return getAvailablePanels().filter(panel => panel.isSelected);
202202
}, [getAvailablePanels]);
203203

204-
const getAvailableColumns = React.useCallback(
205-
(isSidePanel = false) => {
206-
return model.columns.filter(
207-
col =>
208-
(!isSidePanel || !col.isCommon) &&
209-
(isConnectionTracking() || ![ColumnsId.recordtype, ColumnsId.hashid].includes(col.id)) &&
210-
(!col.feature || model.config.features.includes(col.feature))
211-
);
212-
},
213-
[model.columns, model.config.features, isConnectionTracking]
214-
);
204+
const getAvailableColumns = React.useCallback(() => {
205+
return model.columns.filter(
206+
col =>
207+
(isConnectionTracking() || ![ColumnsId.recordtype, ColumnsId.hashid].includes(col.id)) &&
208+
(!col.feature || model.config.features.includes(col.feature))
209+
);
210+
}, [model.columns, model.config.features, isConnectionTracking]);
215211

216212
const getSelectedColumns = React.useCallback(() => {
217213
return getAvailableColumns().filter(column => column.isSelected);
@@ -911,7 +907,7 @@ export const NetflowTraffic: React.FC<NetflowTrafficProps> = ({
911907
allowedScopes={getAllowedScopes()}
912908
canSwitchTypes={isFlow() && isConnectionTracking()}
913909
clearSelections={clearSelections}
914-
availableColumns={getAvailableColumns(true)}
910+
availableColumns={getAvailableColumns()}
915911
maxChunkAge={model.config.maxChunkAgeMs}
916912
selectedColumns={getSelectedColumns()}
917913
/>

web/src/components/query-summary/__tests__/summary-panel.spec.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,13 @@ describe('<SummaryPanel />', () => {
3737
const wrapper = mount(<SummaryPanelContent {...mocks} />);
3838

3939
expect(wrapper.find(Accordion)).toHaveLength(1);
40-
expect(wrapper.find(AccordionItem)).toHaveLength(3);
40+
expect(wrapper.find(AccordionItem)).toHaveLength(5);
4141

4242
expect(wrapper.find('#addresses').last().text()).toBe('5 IP(s)');
4343
expect(wrapper.find('#ports').last().text()).toBe('4 Port(s)');
4444
expect(wrapper.find('#protocols').last().text()).toBe('1 Protocol(s)');
45+
expect(wrapper.find('#Pod').last().text()).toBe('2 Pod(s)');
46+
expect(wrapper.find('#Namespace').last().text()).toBe('1 Namespace(s)');
4547
});
4648

4749
it('should toggle panel', async () => {

web/src/utils/__tests__/columns.spec.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ describe('Columns', () => {
4545
expect(value).toEqual({ kind: 'Pod', name: 'client', namespace: 'foo', showNamespace: false });
4646
});
4747

48-
it('should calculate k8s service name when empty', () => {
48+
it('should calculate k8s owner name when empty', () => {
4949
const col = defColumns.find(c => c.id === ('DstK8S_OwnerName' as ColumnsId));
5050
expect(col).toBeDefined();
5151
const value = col?.value!(flow);
52-
expect(value).toEqual(undefined);
52+
expect(value).toBeUndefined();
5353
});
5454

5555
it('should calculate k8s namespace values', () => {
@@ -90,4 +90,11 @@ describe('Columns', () => {
9090
{ kind: 'Service', name: 'server', namespace: 'bar', showNamespace: true }
9191
]);
9292
});
93+
94+
it('should calculate src+dst K8S owner objects when empty', () => {
95+
const col = defColumns.find(c => c.id === ('K8S_OwnerObject' as ColumnsId));
96+
expect(col).toBeDefined();
97+
const value = col?.value!(flow);
98+
expect(value).toEqual([undefined, undefined]);
99+
});
93100
});

web/src/utils/column-parser.ts

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,29 @@ const forceType = (id: ColumnsId, value: ColValue, type?: FieldType): ColValue =
104104
};
105105

106106
export type ValueFunc = (record: Record) => ColValue;
107+
108+
export const fromFieldFunc = (
109+
def: ColumnConfigDef,
110+
fields: FieldConfig[] | undefined,
111+
field: FieldConfig | undefined
112+
): ValueFunc | undefined => {
113+
if (fields) {
114+
return (r: Record) => {
115+
const result: ColValue[] = fields.map(fc => {
116+
const value = getRecordValue(r, fc.name, undefined);
117+
return forceType(def.id as ColumnsId, value, fc.type);
118+
});
119+
return result.flatMap(r => r) as ColValue;
120+
};
121+
} else if (field) {
122+
return (r: Record) => {
123+
const value = getRecordValue(r, field!.name, '');
124+
return forceType(def.id as ColumnsId, value, field!.type);
125+
};
126+
}
127+
return undefined;
128+
};
129+
107130
export const computeValueFunc = (
108131
def: ColumnConfigDef,
109132
columns: Column[],
@@ -128,20 +151,11 @@ export const computeValueFunc = (
128151
}
129152
return undefined;
130153
};
131-
} else if (fields) {
132-
return (r: Record) => {
133-
const result: ColValue[] = fields.map(fc => {
134-
const value = getRecordValue(r, fc.name, undefined);
135-
return forceType(def.id as ColumnsId, value, fc.type);
136-
});
137-
return result.flatMap(r => r) as ColValue;
138-
};
139-
} else if (field) {
140-
return (r: Record) => {
141-
const value = getRecordValue(r, field!.name, '');
142-
return forceType(def.id as ColumnsId, value, field!.type);
143-
};
144154
}
145-
console.warn('column.value called on ' + def.id + ' but not configured');
146-
return undefined;
155+
156+
const fromField = fromFieldFunc(def, fields, field);
157+
if (fromField === undefined) {
158+
console.warn('column.value called on ' + def.id + ' but not configured');
159+
}
160+
return fromField;
147161
};

web/src/utils/columns.ts

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Record } from '../api/ipfix';
33
import { Feature } from '../model/config';
44
import { FilterId } from '../model/filters';
55
import { compareNumbers, compareStrings } from './base-compare';
6-
import { computeValueFunc } from './column-parser';
6+
import { computeValueFunc, fromFieldFunc } from './column-parser';
77
import { FieldConfig } from './fields';
88
import { compareIPs } from './ip';
99
import { comparePorts } from './port';
@@ -119,6 +119,7 @@ export interface Column {
119119
isSelected: boolean;
120120
isCommon?: boolean;
121121
value?: (flow: Record) => ColValue;
122+
fieldValue?: (flow: Record) => ColValue;
122123
sort(a: Record, b: Record, col: Column): number;
123124
// width in "em"
124125
width: number;
@@ -226,31 +227,29 @@ export const getDefaultColumns = (columnDefs: ColumnConfigDef[], fieldConfigs: F
226227
docURL: !_.isEmpty(d.docURL) ? d.docURL : undefined,
227228
quickFilter: !_.isEmpty(d.filter) ? (d.filter as FilterId) : undefined,
228229
isSelected: d.default === true,
229-
isCommon: !_.isEmpty(d.calculated),
230+
isCommon: d.calculated !== undefined && d.calculated.startsWith('['),
230231
value: computeValueFunc(d, columns, fields, field),
232+
fieldValue: fromFieldFunc(d, fields, field),
231233
sort: (a: Record, b: Record, col: Column) => {
232-
if (d.calculated) {
234+
if (!col.fieldValue) {
233235
return -1;
234-
} else {
235-
if (col.value) {
236-
const valA = col.value(a);
237-
const valB = col.value(b);
238-
if (typeof valA === 'number' && typeof valB === 'number') {
239-
if (col.id.includes('Port')) {
240-
return comparePorts(valA, valB);
241-
} else if (col.id.includes('Proto')) {
242-
return compareProtocols(valA, valB);
243-
}
244-
return compareNumbers(valA, valB);
245-
} else if (typeof valA === 'string' && typeof valB === 'string') {
246-
if (col.id.includes('IP')) {
247-
return compareIPs(valA, valB);
248-
}
249-
return compareStrings(valA, valB);
250-
}
236+
}
237+
const valA = col.fieldValue(a);
238+
const valB = col.fieldValue(b);
239+
if (typeof valA === 'number' && typeof valB === 'number') {
240+
if (col.id.includes('Port')) {
241+
return comparePorts(valA, valB);
242+
} else if (col.id.includes('Proto')) {
243+
return compareProtocols(valA, valB);
244+
}
245+
return compareNumbers(valA, valB);
246+
} else if (typeof valA === 'string' && typeof valB === 'string') {
247+
if (col.id.includes('IP')) {
248+
return compareIPs(valA, valB);
251249
}
252-
return 0;
250+
return compareStrings(valA, valB);
253251
}
252+
return 0;
254253
},
255254
width: d.width || 15,
256255
feature: d.feature

0 commit comments

Comments
 (0)