[Fleet] Fix missing package-level vars in GCP policy editor (elastic#…

…132068) * Fix missing package-level vars in GCP policy editor Display package-level variables even if they aren't defined on the existing package policy during an upgrade, honoring default values for all variables if they exist. Fixes elastic#131251 * Fix not persisting default values for new variables * Fix tests * Address PR feedback + update tests
kpollich · May 16, 2022 · 5fff510 · 5fff510
1 parent 44fb932
commit 5fff510
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 3 deletions.
diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts
@@ -241,6 +241,7 @@ describe('Fleet - validatePackagePolicy()', () => {
           ],
         },
       ],
+      vars: {},
     };
 
     const invalidPackagePolicy: NewPackagePolicy = {
@@ -332,6 +333,7 @@ describe('Fleet - validatePackagePolicy()', () => {
           ],
         },
       ],
+      vars: {},
     };
 
     const noErrorsValidationResults = {
@@ -370,6 +372,7 @@ describe('Fleet - validatePackagePolicy()', () => {
           vars: { 'var-name': null },
         },
       },
+      vars: {},
     };
 
     it('returns no errors for valid package policy', () => {
@@ -416,6 +419,7 @@ describe('Fleet - validatePackagePolicy()', () => {
             streams: { 'with-no-stream-vars-bar': {} },
           },
         },
+        vars: {},
       });
     });
 
@@ -487,6 +491,7 @@ describe('Fleet - validatePackagePolicy()', () => {
             streams: { 'with-no-stream-vars-bar': {} },
           },
         },
+        vars: {},
       });
     });
 
@@ -505,6 +510,7 @@ describe('Fleet - validatePackagePolicy()', () => {
         description: null,
         namespace: null,
         inputs: null,
+        vars: {},
       });
       expect(
         validatePackagePolicy(
@@ -520,6 +526,7 @@ describe('Fleet - validatePackagePolicy()', () => {
         description: null,
         namespace: null,
         inputs: null,
+        vars: {},
       });
     });
 
@@ -538,6 +545,7 @@ describe('Fleet - validatePackagePolicy()', () => {
         description: null,
         namespace: null,
         inputs: null,
+        vars: {},
       });
       expect(
         validatePackagePolicy(
@@ -553,6 +561,7 @@ describe('Fleet - validatePackagePolicy()', () => {
         description: null,
         namespace: null,
         inputs: null,
+        vars: {},
       });
     });
 
@@ -604,6 +613,7 @@ describe('Fleet - validatePackagePolicy()', () => {
             },
           },
         },
+        vars: {},
       });
     });
 
@@ -729,6 +739,7 @@ describe('Fleet - validatePackagePolicy()', () => {
             },
           },
         },
+        vars: {},
         name: null,
         namespace: null,
       });

diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.ts
@@ -55,6 +55,7 @@ export const validatePackagePolicy = (
     description: null,
     namespace: null,
     inputs: {},
+    vars: {},
   };
   const namespaceValidation = isValidNamespace(packagePolicy.namespace);
 

diff --git a/...leet/sections/agent_policy/create_package_policy_page/step_define_package_policy.test.tsx b/...leet/sections/agent_policy/create_package_policy_page/step_define_package_policy.test.tsx
@@ -82,11 +82,17 @@ describe('StepDefinePackagePolicy', () => {
     };
   });
 
-  const validationResults = { name: null, description: null, namespace: null, inputs: {} };
+  const validationResults = {
+    name: null,
+    description: null,
+    namespace: null,
+    inputs: {},
+    vars: {},
+  };
 
   let testRenderer: TestRenderer;
   let renderResult: ReturnType<typeof testRenderer.render>;
-  const render = () =>
+  const render = ({ isUpdate } = { isUpdate: false }) =>
     (renderResult = testRenderer.render(
       <StepDefinePackagePolicy
         agentPolicy={agentPolicy}
@@ -95,6 +101,7 @@ describe('StepDefinePackagePolicy', () => {
         updatePackagePolicy={mockUpdatePackagePolicy}
         validationResults={validationResults}
         submitAttempted={false}
+        isUpdate={isUpdate}
       />
     ));
 
@@ -199,4 +206,23 @@ describe('StepDefinePackagePolicy', () => {
       expect(renderResult.getByDisplayValue('apache-11')).toBeInTheDocument();
     });
   });
+
+  describe('update', () => {
+    describe('when package vars are introduced in a new package version', () => {
+      it('should display new package vars', () => {
+        render({ isUpdate: true });
+
+        waitFor(async () => {
+          expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
+          expect(renderResult.getByText('Required var')).toBeInTheDocument();
+
+          await act(async () => {
+            fireEvent.click(renderResult.getByText('Advanced options').closest('button')!);
+          });
+
+          expect(renderResult.getByText('Advanced var')).toBeInTheDocument();
+        });
+      });
+    });
+  });
 });
diff --git a/...ons/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx b/...ons/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx
@@ -90,9 +90,28 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{
 
     // Update package policy's package and agent policy info
     useEffect(() => {
-      if (isUpdate || isLoadingPackagePolicies) {
+      if (isLoadingPackagePolicies) {
         return;
       }
+
+      if (isUpdate) {
+        // If we're upgrading, we need to make sure we catch an addition of package-level
+        // vars when they were previously no package-level vars defined
+        if (!packagePolicy.vars && packageInfo.vars) {
+          updatePackagePolicy(
+            packageToPackagePolicy(
+              packageInfo,
+              agentPolicy?.id || '',
+              packagePolicy.output_id,
+              packagePolicy.namespace,
+              packagePolicy.name,
+              packagePolicy.description,
+              integrationToEnable
+            )
+          );
+        }
+      }
+
       const pkg = packagePolicy.package;
       const currentPkgKey = pkg ? pkgKeyFromPackageInfo(pkg) : '';
       const pkgKey = pkgKeyFromPackageInfo(packageInfo);
@@ -211,6 +230,7 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{
             const { name: varName, type: varType } = varDef;
             if (!packagePolicy.vars || !packagePolicy.vars[varName]) return null;
             const value = packagePolicy.vars[varName].value;
+
             return (
               <EuiFlexItem key={varName}>
                 <PackagePolicyInputVarField