generate: check for existing public key in manifest

[davidweisse]

If you call generate twice with the same seed share owner public key, the public key would just have been appended to the list instead of checking if the public key is already present in the manifest. Now, calling generate twice will result in the same manifest.

[burgerdev]

I'm not sure about this. If I generate twice like this:

contrast generate --seedshare-owner-key=alice.pem deploy.yml
contrast generate --seedshare-owner-key=bob.pem deploy.yml

Would I really expect both alice and bob to be in the manifest? I think it might be less surprising if we cleared out the keys and then added what's given as flag.

[katexochen]

I'm not sure about this. If I generate twice like this:

contrast generate --seedshare-owner-key=alice.pem deploy.yml
contrast generate --seedshare-owner-key=bob.pem deploy.yml

Would I really expect both alice and bob to be in the manifest? I think it might be less surprising if we cleared out the keys and then added what's given as flag.

We should match the behavior of workload owner keys (or update it, too).

[davidweisse]

I'm not sure about this. If I generate twice like this:

contrast generate --seedshare-owner-key=alice.pem deploy.yml
contrast generate --seedshare-owner-key=bob.pem deploy.yml

Would I really expect both alice and bob to be in the manifest? I think it might be less surprising if we cleared out the keys and then added what's given as flag.

This is the current behavior for workload owner keys, as far as I can see.

[Freax13]

I'm not sure about this. If I generate twice like this:

contrast generate --seedshare-owner-key=alice.pem deploy.yml
contrast generate --seedshare-owner-key=bob.pem deploy.yml

Would I really expect both alice and bob to be in the manifest? I think it might be less surprising if we cleared out the keys and then added what's given as flag.

We could rename the flag to something like --add-seedshare-owner-key to make it more explicit to the user that we only add keys and don't clear them.

[katexochen]

We could rename the flag to something like --add-seedshare-owner-key to make it more explicit to the user that we only add keys and don't clear them.

I say that's a good solution, wdyt @burgerdev ?

[burgerdev]

Calling the flags --add-*-key should manage expectations, I agree, so if there's no concrete use-case for idempotency of generate runs I won't object. When we get to actually implement seed sharing, I wonder what the UX would look like for many keys, though - maybe users would rather modify the manifest directly?

[Freax13]

I also applied the change suggested in #670 (comment) to the add-seedshare-owner-key flag here.