fix: use temp dir for secret data (#1290)

Change the mechanism used to pass secret values from ops to hook tools
from command line to files in temporary directory.

Removes the 2MB total secret size limit imposed by command line length

Hook tool calls juju agent which in turn reads these files, I've
confirmed with the Juju team that juju agent and charm code are run as
the same user.

ops/model.py

@@ -3598,11 +3598,13 @@ def secret_set(
             args.extend(['--expire', expire.isoformat()])
         if rotate is not None:
             args += ['--rotate', rotate.value]
-        if content is not None:
-            # The content has already been validated with Secret._validate_content
-            for k, v in content.items():
-                args.append(f'{k}={v}')
-        self._run_for_secret('secret-set', *args)
+        with tempfile.TemporaryDirectory() as tmp:
+            # The content is None or has already been validated with Secret._validate_content
+            for k, v in (content or {}).items():
+                with open(f'{tmp}/{k}', mode='w', encoding='utf-8') as f:
+                    f.write(v)
+                args.append(f'{k}#file={tmp}/{k}')
+            self._run_for_secret('secret-set', *args)
 
     def secret_add(
         self,
@@ -3625,10 +3627,13 @@ def secret_add(
             args += ['--rotate', rotate.value]
         if owner is not None:
             args += ['--owner', owner]
-        # The content has already been validated with Secret._validate_content
-        for k, v in content.items():
-            args.append(f'{k}={v}')
-        result = self._run('secret-add', *args, return_output=True)
+        with tempfile.TemporaryDirectory() as tmp:
+            # The content has already been validated with Secret._validate_content
+            for k, v in content.items():
+                with open(f'{tmp}/{k}', mode='w', encoding='utf-8') as f:
+                    f.write(v)
+                args.append(f'{k}#file={tmp}/{k}')
+            result = self._run('secret-add', *args, return_output=True)
         secret_id = typing.cast(str, result)
         return secret_id.strip()
 

test/test_helpers.py

@@ -153,6 +153,15 @@ def write(self, name: str, content: str):
             f.write(
                 """#!/bin/sh
 {{ printf {name}; printf "\\036%s" "$@"; printf "\\034"; }} >> {path}/calls.txt
+
+# Capture key and data from key#file=/some/path arguments
+for word in "$@"; do
+  echo "$word" | grep -q "#file=" || continue
+  key=$(echo "$word" | cut -d'#' -f1)
+  path=$(echo "$word" | cut -d'=' -f2)
+  cp "$path" "{path}/$key.secret"
+done
+
 {content}""".format_map(template_args)
             )
         path.chmod(0o755)
@@ -175,6 +184,9 @@ def calls(self, clear: bool = False) -> typing.List[typing.List[str]]:
                 f.truncate(0)
         return calls
 
+    def secrets(self) -> typing.Dict[str, str]:
+        return {p.stem: p.read_text() for p in self.path.iterdir() if p.suffix == '.secret'}
+
 
 class FakeScriptTest(unittest.TestCase):
     def test_fake_script_works(self):

test/test_model.py

@@ -24,7 +24,7 @@
 import unittest
 from collections import OrderedDict
 from textwrap import dedent
-from unittest.mock import MagicMock, patch
+from unittest.mock import ANY, MagicMock, patch
 
 import pytest
 
@@ -3349,7 +3349,8 @@ def test_app_add_secret_simple(self, fake_script: FakeScript, model: ops.Model):
         assert secret.id == 'secret:123'
         assert secret.label is None
 
-        assert fake_script.calls(clear=True) == [['secret-add', '--owner', 'application', 'foo=x']]
+        assert fake_script.calls(clear=True) == [['secret-add', '--owner', 'application', ANY]]
+        assert fake_script.secrets() == {'foo': 'x'}
 
     def test_app_add_secret_args(self, fake_script: FakeScript, model: ops.Model):
         fake_script.write('secret-add', 'echo secret:234')
@@ -3379,10 +3380,11 @@ def test_app_add_secret_args(self, fake_script: FakeScript, model: ops.Model):
                 'hourly',
                 '--owner',
                 'application',
-                'foo=x',
-                'bar=y',
+                ANY,
+                ANY,
             ]
         ]
+        assert fake_script.secrets() == {'foo': 'x', 'bar': 'y'}
 
     def test_unit_add_secret_simple(self, fake_script: FakeScript, model: ops.Model):
         fake_script.write('secret-add', 'echo secret:345')
@@ -3392,7 +3394,8 @@ def test_unit_add_secret_simple(self, fake_script: FakeScript, model: ops.Model)
         assert secret.id == 'secret:345'
         assert secret.label is None
 
-        assert fake_script.calls(clear=True) == [['secret-add', '--owner', 'unit', 'foo=x']]
+        assert fake_script.calls(clear=True) == [['secret-add', '--owner', 'unit', ANY]]
+        assert fake_script.secrets() == {'foo': 'x'}
 
     def test_unit_add_secret_args(self, fake_script: FakeScript, model: ops.Model):
         fake_script.write('secret-add', 'echo secret:456')
@@ -3422,10 +3425,11 @@ def test_unit_add_secret_args(self, fake_script: FakeScript, model: ops.Model):
                 'yearly',
                 '--owner',
                 'unit',
-                'foo=w',
-                'bar=z',
+                ANY,
+                ANY,
             ]
         ]
+        assert fake_script.secrets() == {'foo': 'w', 'bar': 'z'}
 
     def test_unit_add_secret_errors(self, model: ops.Model):
         # Additional add_secret tests are done in TestApplication
@@ -3721,10 +3725,11 @@ def test_set_content(self, model: ops.Model, fake_script: FakeScript):
             secret.set_content({'s': 't'})  # ensure it validates content (key too short)
 
         assert fake_script.calls(clear=True) == [
-            ['secret-set', 'secret:x', 'foo=bar'],
+            ['secret-set', 'secret:x', ANY],
             ['secret-info-get', '--label', 'y', '--format=json'],
-            ['secret-set', 'secret:z', 'bar=foo'],
+            ['secret-set', 'secret:z', ANY],
         ]
+        assert fake_script.secrets() == {'foo': 'bar', 'bar': 'foo'}
 
     def test_set_info(self, model: ops.Model, fake_script: FakeScript):
         fake_script.write('secret-set', """exit 0""")