Improve playground-sign output

[jku]

The tool now mostly describes changes to roles metadata. There are still some gaps:

• This should be accompanied with strict validation (which allows us to not describe the things we validate)
• The key content changes are not yet handled

The code is pretty ugly... but I think it's partly unavoidable: even after better refactoring this is likely a long list of special cases. That said, there's definitely room for refactoring. I'd like to merge this and move on for now but I'll take opinions on this.

[jku]

This is also not tested in any way: Testing for CLI output is a bit tricky -- maybe testing for the still missing validation makes more sense

[kommendorkapten]

Did some smoke test, and found one issue. I had to apply this hunk to make it work:

--- a/playground/signer/playground_sign/_signer_repository.py
+++ b/playground/signer/playground_sign/_signer_repository.py
@@ -551,7 +551,8 @@ class SignerRepository(Repository):
                 old_role = old_delegations[name]
                 old_signers = []
                 for key in self._get_keys(name, known_good=True):
-                    old_signers.append(key.unrecognized_fields["x-playground-keyowner"])
+                    if "x-playground-keyowner" in key.unrecognized_fields:
+                        old_signers.append(key.unrecognized_fields["x-playground-keyowner"])

                 if role != old_role:
                     output.append(f" * Modified {title}")

[jku]

Did some smoke test, and found one issue. I had to apply this hunk to make it work:

--- a/playground/signer/playground_sign/_signer_repository.py
+++ b/playground/signer/playground_sign/_signer_repository.py
@@ -551,7 +551,8 @@ class SignerRepository(Repository):
                 old_role = old_delegations[name]
                 old_signers = []
                 for key in self._get_keys(name, known_good=True):
-                    old_signers.append(key.unrecognized_fields["x-playground-keyowner"])
+                    if "x-playground-keyowner" in key.unrecognized_fields:
+                        old_signers.append(key.unrecognized_fields["x-playground-keyowner"])

                 if role != old_role:
                     output.append(f" * Modified {title}")

oh yeah: I handled the online role as a special case just 10 lines earlier but forgot to do it here : the online key does not have "keyowner" (I wonder if it would make sense for it to have that field... it would make things like this easier)

[jku]

Did some smoke test...

Do you remember how you triggered this? I'm trying to see if I can make a test but I realised modifying online delegations has another issue (#100) so you probably were not doing that...

[kommendorkapten]

Do you remember how you triggered this? I'm trying to see if I can make a test but I realised modifying online delegations has another issue (https://github.com/jku/repository-playground/issues/100) so you probably were not doing that...

Yes.

1. run playground-delegate add--foo
2. Add new signer (-foo)
3. As -foo run playground-sign add--foo
4. As -foo run playground-sign add--foo again to actually sign the metadata

% playground-sign add--foo
Signing event add--foo (commit f89310e)
Your signature is requested for role(s) ['root'].
Traceback (most recent call last):
  File "/opt/homebrew/bin/playground-sign", line 8, in <module>
    sys.exit(sign())
             ^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kommendorkapten/git/repository-playground/playground/signer/playground_sign/sign.py", line 63, in sign
    click.echo(repo.status(rolename))
               ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kommendorkapten/git/repository-playground/playground/signer/playground_sign/_signer_repository.py", line 595, in status
    output.extend(self._delegation_status_lines(rolename))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kommendorkapten/git/repository-playground/playground/signer/playground_sign/_signer_repository.py", line 554, in _delegation_status_lines
    old_signers.append(key.unrecognized_fields["x-playground-keyowner"])
                       ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'x-playground-keyowner'
% 

[jku]

ah, so this is likely somehow related to the issue that forces you to run sign twice: I will try reproduce that today to understand what happens

[jku]

This is now based on top of #104, keeping draft until that one is merged

[kommendorkapten]

Tests conducted:

• Init a new TUF repo - ✅
• Add/invite a new signer - ✅
• Add targets - ✅

The output is significantly better now 🥳

Will continue with the code review now.