Fix malformed feature flag support (#17327)

Author: nwmac

shell/list/__tests__/management.cattle.io.feature.test.ts

@@ -0,0 +1,105 @@
+import { shallowMount } from '@vue/test-utils';
+
+jest.mock('@shell/mixins/resource-fetch', () => ({
+  __esModule: true,
+  default:    {
+    data() {
+      return {
+        forceUpdateLiveAndDelayed: 0, loading: false, rows: []
+      };
+    },
+    async $fetchType() {}
+  }
+}));
+
+// eslint-disable-next-line import/first
+import ManagementFeature from '@shell/list/management.cattle.io.feature.vue';
+
+const createMockStore = () => ({
+  getters: {
+    'i18n/t':               (key: string) => key,
+    'management/schemaFor': () => ({ resourceMethods: ['PUT'] }),
+  },
+  dispatch: jest.fn(),
+});
+
+const createWrapper = (rows: any[]) => {
+  return shallowMount(ManagementFeature, {
+    props: {
+      resource: 'management.cattle.io.feature',
+      schema:   { id: 'management.cattle.io.feature' } as any,
+    },
+    data:   () => ({ rows }),
+    global: {
+      mocks: {
+        $store:      createMockStore(),
+        $fetchState: { pending: false },
+        $fetchType:  jest.fn(),
+      },
+      stubs: {
+        // Render the cell:name slot directly so we can assert on the lock icon
+        ResourceTable: {
+          props:    ['rows'],
+          template: '<div><slot name="cell:name" :row="rows[0]" /></div>',
+        },
+      },
+    }
+  });
+};
+
+describe('list/management.cattle.io.feature', () => {
+  describe('locked icon rendering in cell:name slot', () => {
+    it('should render the lock icon when status.lockedValue is not null', () => {
+      const row = {
+        metadata:    { name: 'feature-a' },
+        nameDisplay: 'feature-a',
+        status:      { lockedValue: true },
+      };
+
+      const wrapper = createWrapper([row]);
+
+      expect(wrapper.find('i.icon-lock').exists()).toBe(true);
+    });
+
+    it('should not render the lock icon when status.lockedValue is null', () => {
+      const row = {
+        metadata:    { name: 'feature-a' },
+        nameDisplay: 'feature-a',
+        status:      { lockedValue: null },
+      };
+
+      const wrapper = createWrapper([row]);
+
+      expect(wrapper.find('i.icon-lock').exists()).toBe(false);
+    });
+
+    it('should not throw and should not render the lock icon when status is missing (malformed feature flag)', () => {
+      const row = {
+        metadata:    { name: 'feature-a' },
+        nameDisplay: 'feature-a',
+      };
+
+      expect(() => createWrapper([row])).not.toThrow();
+
+      const wrapper = createWrapper([row]);
+
+      expect(wrapper.find('i.icon-lock').exists()).toBe(false);
+    });
+  });
+
+  describe('filteredRows', () => {
+    it('should filter out hidden feature flags', () => {
+      const rows = [
+        { metadata: { name: 'fleet' } },
+        { metadata: { name: 'some-feature' } },
+      ];
+
+      const wrapper = createWrapper(rows);
+
+      const filtered = (wrapper.vm as any).filteredRows;
+
+      expect(filtered).toHaveLength(1);
+      expect(filtered[0].metadata.name).toBe('some-feature');
+    });
+  });
+});

shell/list/management.cattle.io.feature.vue

@@ -69,7 +69,7 @@ export default {
         <div class="feature-name">
           <div>{{ scope.row.nameDisplay }}</div>
           <i
-            v-if="scope.row.status.lockedValue !== null"
+            v-if="scope.row.status && scope.row.status.lockedValue !== null"
             class="icon icon-lock"
           />
         </div>

shell/models/__tests__/management.cattle.io.feature.test.ts

@@ -0,0 +1,131 @@
+import Feature from '@shell/models/management.cattle.io.feature.js';
+import Resource from '@shell/plugins/dashboard-store/resource-class';
+
+describe('class Feature', () => {
+  const ctx = {
+    dispatch:    jest.fn(),
+    rootGetters: { 'i18n/t': (key: string) => key },
+    getters:     { schemaFor: () => ({ linkFor: jest.fn() }) },
+  };
+
+  // The parent Resource._availableActions getter depends on runtime config we don't have
+  // in tests — stub it out so we can assert on Feature's own additions.
+  beforeEach(() => {
+    jest.spyOn(Resource.prototype, '_availableActions', 'get').mockReturnValue([]);
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  describe('enabled getter', () => {
+    it.each([
+      [true, true],
+      [false, false],
+    ])('should return lockedValue (%s) when status.lockedValue is not null', (lockedValue, expected) => {
+      const feature = new Feature({
+        spec:   { value: false },
+        status: { lockedValue, default: false }
+      }, ctx);
+
+      expect(feature.enabled).toBe(expected);
+    });
+
+    it('should return spec.value when lockedValue is null and spec.value is set', () => {
+      const feature = new Feature({
+        spec:   { value: true },
+        status: { lockedValue: null, default: false }
+      }, ctx);
+
+      expect(feature.enabled).toBe(true);
+    });
+
+    it('should fall back to status.default when lockedValue is null and spec.value is null', () => {
+      const feature = new Feature({
+        spec:   { value: null },
+        status: { lockedValue: null, default: true }
+      }, ctx);
+
+      expect(feature.enabled).toBe(true);
+    });
+
+    it('should not throw when status is missing (malformed feature flag)', () => {
+      const feature = new Feature({ spec: { value: true } }, ctx);
+
+      expect(() => feature.enabled).not.toThrow();
+      expect(feature.enabled).toBe(true);
+    });
+  });
+
+  describe('restartRequired getter', () => {
+    it('should return false when status.dynamic is true', () => {
+      const feature = new Feature({ spec: {}, status: { dynamic: true, lockedValue: null } }, ctx);
+
+      expect(feature.restartRequired).toBe(false);
+    });
+
+    it('should return true when status.dynamic is false', () => {
+      const feature = new Feature({ spec: {}, status: { dynamic: false, lockedValue: null } }, ctx);
+
+      expect(feature.restartRequired).toBe(true);
+    });
+
+    it('should return true when status is missing (malformed feature flag)', () => {
+      const feature = new Feature({ spec: {} }, ctx);
+
+      expect(() => feature.restartRequired).not.toThrow();
+      expect(feature.restartRequired).toBe(true);
+    });
+  });
+
+  describe('_availableActions getter', () => {
+    it('should disable the toggle action when lockedValue is not null', () => {
+      const feature = new Feature({
+        spec:   { value: false },
+        status: {
+          lockedValue: true, default: false, dynamic: true
+        },
+      }, ctx);
+
+      jest.spyOn(feature, 'canUpdate', 'get').mockReturnValue(true);
+
+      const actions = feature._availableActions;
+
+      expect(actions[0].action).toBe('toggleFeatureFlag');
+      expect(actions[0].enabled).toBe(false);
+    });
+
+    it('should enable the toggle action when lockedValue is null and user canUpdate', () => {
+      const feature = new Feature({
+        spec:   { value: false },
+        status: {
+          lockedValue: null, default: false, dynamic: true
+        },
+        id: 'some-feature',
+      }, ctx);
+
+      jest.spyOn(feature, 'canUpdate', 'get').mockReturnValue(true);
+
+      const actions = feature._availableActions;
+
+      expect(actions[0].action).toBe('toggleFeatureFlag');
+      expect(actions[0].enabled).toBe(true);
+    });
+
+    it('should not throw and should disable the toggle action when status is missing (malformed feature flag)', () => {
+      const feature = new Feature({
+        spec: { value: false },
+        id:   'some-feature',
+      }, ctx);
+
+      jest.spyOn(feature, 'canUpdate', 'get').mockReturnValue(true);
+
+      expect(() => feature._availableActions).not.toThrow();
+
+      const actions = feature._availableActions;
+
+      expect(actions[0].action).toBe('toggleFeatureFlag');
+      expect(actions[0].enabled).toBeFalsy();
+    });
+  });
+});

shell/models/management.cattle.io.feature.js

@@ -8,7 +8,7 @@ export default class Feature extends HybridModel {
 
   get enabled() {
     // If lockedValue is not null, then this is the value that the flag is locked to, so that should be used
-    if (this.status.lockedValue !== null) {
+    if (this.status && this.status.lockedValue !== null) {
       return this.status.lockedValue;
     }
 
@@ -17,7 +17,7 @@ export default class Feature extends HybridModel {
   }
 
   get restartRequired() {
-    return !this.status.dynamic;
+    return !this.status?.dynamic;
   }
 
   get canYaml() {
@@ -43,7 +43,7 @@ export default class Feature extends HybridModel {
     // User can not disable or enable if the feature flag is locked
     // Note: lockedValue is the value that the feature flag is locked to, so it can be true or false
     // It can also be null, which indicates that the feature flag is not locked
-    enableAction.enabled = enableAction.enabled && (this.status.lockedValue === null);
+    enableAction.enabled = enableAction.enabled && (this.status && this.status.lockedValue === null);
 
     out.unshift(enableAction);