Checksum type publication check #1086
Conversation
pavelpicka
force-pushed
the
7988-publication-fips
branch
from
5ddbc2d
to
fa92d5b
Compare
|
Attached issue: https://pulp.plan.io/issues/7988 |
pavelpicka
force-pushed
the
7988-publication-fips
branch
6 times, most recently
from
5481dc3
to
80a4533
Compare
|
Introduced part of error message we can re-use to point user where to update setting about checksum types. |
pavelpicka
force-pushed
the
7988-publication-fips
branch
from
80a4533
to
83c695e
Compare
|
I'd highly encourage some testing on this PR since the logic is complicated. You'll probably have to write a unit test. |
pavelpicka
force-pushed
the
7988-publication-fips
branch
6 times, most recently
from
403b1f7
to
c230684
Compare
pavelpicka
force-pushed
the
7988-publication-fips
branch
from
c230684
to
a46b7cb
Compare
pavelpicka
force-pushed
the
7988-publication-fips
branch
2 times, most recently
from
5575db1
to
d00cfda
Compare
|
@bmbouter, @daviddavis, @ipanova PR back to ready for re-review |
|
Three small comments. Other than that, I think this is ready to merge. |
pavelpicka
force-pushed
the
7988-publication-fips
branch
from
d00cfda
to
f8865a9
Compare
User receives error when try to publish content does not contain at least on artifact with disallowed checksum type. closes: #7988 https://pulp.plan.io/issues/7988
pavelpicka
force-pushed
the
7988-publication-fips
branch
from
f8865a9
to
65b3198
Compare
| @@ -0,0 +1 @@ | |||
| Publication creation will fail when there is content which doesn't contain at least one artifact with allowed checksum. | |||
There was a problem hiding this comment.
@daviddavis when combining with this PR #1156 do i read correctly that publish will succeed with 1 forbidden( for example md5) and 1allowed checksum ( sha256) but when we get to streaming the artifact it will fail in any case because that remote artifact has forbidden checksum?
There was a problem hiding this comment.
Yes, exactly. The only thing I would mention is that it should continue to the next remote artifact and try to stream it if the first one fails.
|
There is one edge case I thought of. Do plugins ever create RemoteArtifacts with no checksums? Looks like it's possible and I believe that certain plugins would have to if they do not have checksums in the metadata. If this is the case, then this code wouldn't work properly as you could never create a publication for these RemoteArtifacts. If I am correct here, then I think we should probably just reject any publication that would have a RemoteArtifact with a forbidden checksum. |
|
Rubygems do not have checksums. (Last time i checked.) |
|
In our FIPS meeting today, we decided to postpone this work indefinitely (until a user requests it or we deem it necessary) and instead just rely on rejecting on-demand content during sync and in the content app before saving it. |
User receive error when try to publish content with disallowed checksum type.
closes: #7988
https://pulp.plan.io/issues/7988