Delegations with Managed Secondaries

[jsrc27]

Metadata verification was added to Managed Secondaries via: #35

However, this introduced a limitation concerning delegated targets. The final step of the verification matches director targets to image-repo targets. But, the function used to do this matching DirectorRepository::matchTargetsWithImageTargets(), does not support delegations and only checks the top-level targets metadata as seen here: https://github.com/uptane/aktualizr/blob/master/src/libaktualizr/uptane/directorrepository.cc#L157

Therefore if any of the targets are located in delegated metadata then the update will fail due to verification error from the secondary. Even if the delegated target is not for the secondary.

CC: @tkfu & @cajun-rat

[pattivacek]

However, this introduced a limitation concerning delegated targets.

Well, sort of, yes. Before that there was no metadata verification whatsoever. I mean, the managed Secondaries exist primarily for demonstration and testing purposes, so it originally wasn't deemed necessary that the metadata would be reverified, since the idea more or less was that the Primary was already verifying it, and the "managed" aspect of the Secondary meant that it couldn't verify metadata itself, so it was only the responsibility of the Primary to verify ("manage") it. In fact, by adding that logic, I've created what is arguably unnecessarily duplicated verification. However, it was convenient for me for testing purposes, so I did it.

I have the impression that there are, in fact, some users of managed Secondaries in production, which is fine and good. However, I also have the impression that there are not any users of delegated Targets metadata. To my knowledge, that implementation was never completed. It might work for the Primary, but I wouldn't count on it without doing a lot of testing.

If you are suggesting that you have a need for this support, I would welcome PRs. It shouldn't be that hard to do, but it wasn't something that I was interested in when I was working on #35.

[jsrc27]

However, I also have the impression that there are not any users of delegated Targets metadata.

Actually in our setup we use delegated targets quite frequently. Which is how we noticed this since we pulled in the change that added metadata verification to managed secondaries.

I mean, the managed Secondaries exist primarily for demonstration and testing purposes

This is interesting. If I understand correctly then, is the managed secondary implementation not meant to be used in production?

Currently we have a custom secondary which heavily derives from the managed secondary class. On top of that some of the functions and logic still come directly from default managed secondary class. Like this metadata verification for example. In this case are we misusing managed secondaries then?

In fact, by adding that logic, I've created what is arguably unnecessarily duplicated verification. However, it was convenient for me for testing purposes, so I did it.

So then these checks in the managed secondary are completely unnecessary, and can be removed in production?

If you are suggesting that you have a need for this support, I would welcome PRs.

We would love to submit something here when we can. I just wanted to have a discussion here first since I can see there was a TODO in the code for this, and perhaps there was some internal conversations about this. I assume the TODO was from the HERE group.

[pattivacek]

Actually in our setup we use delegated targets quite frequently. Which is how we noticed this since we pulled in the change that added metadata verification to managed secondaries.

Oh, great! I'm thrilled to hear that! Can you disclose the organization you work for? I'm just curious, no pressure at all. You can also email me if you'd prefer.

If I understand correctly then, is the managed secondary implementation not meant to be used in production?

Currently we have a custom secondary which heavily derives from the managed secondary class. On top of that some of the functions and logic still come directly from default managed secondary class. Like this metadata verification for example. In this case are we misusing managed secondaries then?

Nah, it's fine! We always knew that some people used virtual/managed Secondaries in production. I should rather say that it was originally intended for development, or at least the VirtualSecondary class specifically. The ManagedSecondary class was actually added so that users could subclass it exactly as you have. aktualizr-secondary was the same way: originally for testing, now also used in production.

So then these checks in the managed secondary are completely unnecessary, and can be removed in production?

I think this is a matter of debate. I don't think they are strictly necessary, but they don't hurt. I think they're a good idea. One option could be to move the metadata verification into the VirtualSecondary class so that ManagedSecondary implementations can choose what they want to do. I'm a bit curious if @tkfu @cajun-rat @mike-sul have opinions here.

We would love to submit something here when we can. I just wanted to have a discussion here first since I can see there was a TODO in the code for this, and perhaps there was some internal conversations about this. I assume the TODO was from the HERE group.

Great, and totally makes sense. Thanks for that! Actually, I think those TODOs all came from me. Some back when I was still working at Here, some from last year when I was adding Secondary Root rotation support for my current employer.

[jsrc27]

Oh, great! I'm thrilled to hear that! Can you disclose the organization you work for? I'm just curious, no pressure at all. You can also email me if you'd prefer.

I'm working for Toradex, same organization as @tkfu.

I think this is a matter of debate. I don't think they are strictly necessary, but they don't hurt. I think they're a good idea. One option could be to move the metadata verification into the VirtualSecondary class so that ManagedSecondary implementations can choose what they want to do.

This would be interesting to discuss. As for the time-being as a temporary "fix" we omitted the check in the secondary to stop it from failing on updates that involved delegated targets. We figured this would be "okay" for now since these checks were added relatively recently and Aktualizr has existed without these checks beforehand.

[pattivacek]

I'm working for Toradex

Ah, okay, great! He also just mentioned an interest in delegations, so now it all makes sense.

As for the time-being as a temporary "fix" we omitted the check in the secondary to stop it from failing on updates that involved delegated targets.

It sounds like you understand the risk, which I agree is minimal.

This would be interesting to discuss.

Since you appear to the first people to actually care about this feature, I'm happy to hear what your needs and preferences are.