Implement enhancements/oc/mirroring-release-signatures #343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
do-not-merge/work-in-progress
openshift-ci-robot
requested review from
deads2k,
LalatenduMohanty,
soltysh,
wking and
smarterclayton
openshift-ci-robot
added
the
needs-rebase
openshift-ci-robot
removed
the
needs-rebase
jottofar
force-pushed
the
ota-119
branch
2 times, most recently
from
87444e7 to
5fe2d40
Compare
|
/test |
|
/retest |
|
/test verify |
|
/test |
|
/retest |
|
/test verify |
|
/test images |
|
/retest |
jottofar
force-pushed
the
ota-119
branch
2 times, most recently
from
e915b74 to
c46131d
Compare
jottofar
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
wking
reviewed
Apr 17, 2020
| } | ||
| for i := range ms { | ||
| ms[i].OriginalFilename = filepath.Base(hdr.Name) | ||
| src := fmt.Sprintf("the config map %s/%s", ms[i].Obj.GetNamespace(), ms[i].Obj.GetName()) |
There was a problem hiding this comment.
Do we know it's a config map here? I don't see you repeating the CVO's GVK check.
There was a problem hiding this comment.
I was suggesting using proper k8s encoding, as a follow-up. That will be the best.
wking
reviewed
Apr 17, 2020
soltysh
approved these changes
Apr 17, 2020
| } | ||
| for i := range ms { | ||
| ms[i].OriginalFilename = filepath.Base(hdr.Name) | ||
| src := fmt.Sprintf("the config map %s/%s", ms[i].Obj.GetNamespace(), ms[i].Obj.GetName()) |
There was a problem hiding this comment.
I was suggesting using proper k8s encoding, as a follow-up. That will be the best.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jottofar, soltysh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
Was this intended to be a breaking change? This broke the e2e-metal-ipi automation. I filed https://bugzilla.redhat.com/show_bug.cgi?id=1825565, including a suggestion if you could just dump the config map to $PWD, instead of failing catastrophically. If this breakage was intentional, some announcement would've been helpful. |
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Only attempt to write to a file when either --release-image-signature-to-dir or --to-dir had been set. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all, and avoids crashing if a default, unasked-for, config directory is not writeable [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Convert handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bother with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Check to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Only attempt to write to a file when either --release-image-signature-to-dir or --to-dir had been set. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all, and avoids crashing if a default, unasked-for, config directory is not writeable [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Only attempt to write to a file when either --release-image-signature-to-dir or --to-dir had been set. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all, and avoids crashing if a default, unasked-for, config directory is not writeable [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Only attempt to write to a file when either --release-image-signature-to-dir or --to-dir had been set. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all, and avoids crashing if a default, unasked-for, config directory is not writeable [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Only attempt to write to a file when either --release-image-signature-to-dir or --to-dir had been set. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all, and avoids crashing if a default, unasked-for, config directory is not writeable [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Only attempt to write to a file when either --release-image-signature-to-dir or --to-dir had been set. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all, and avoids crashing if a default, unasked-for, config directory is not writeable [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
added a commit
to wking/oc
that referenced
this pull request
Apr 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
jottofar
pushed a commit
to jottofar/oc
that referenced
this pull request
May 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
jottofar
pushed a commit
to jottofar/oc
that referenced
this pull request
May 19, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
jottofar
pushed a commit
to jottofar/oc
that referenced
this pull request
May 21, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
jottofar
pushed a commit
to jottofar/oc
that referenced
this pull request
May 29, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
jottofar
pushed a commit
to jottofar/oc
that referenced
this pull request
May 29, 2020
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
jottofar
added a commit
to jottofar/oc
that referenced
this pull request
Jul 8, 2020
Modify to use proper k8s encoding/decoding and other minor cleanup. Reference open comments from openshift#343.
jottofar
added a commit
to jottofar/oc
that referenced
this pull request
Aug 26, 2020
Modify to use proper k8s encoding/decoding and other minor cleanup. Reference open comments from openshift#343. Use library-go verify package which was created since both CVO and the oc client use them.
jottofar
added a commit
to jottofar/oc
that referenced
this pull request
Aug 26, 2020
Modify to use proper k8s encoding/decoding and other minor cleanup. Reference open comments from openshift#343. Use library-go verify package which was created since both CVO and the oc client use them.
jottofar
added a commit
to jottofar/oc
that referenced
this pull request
Sep 9, 2020
Modify to use proper k8s encoding/decoding and other minor cleanup. Reference open comments from openshift#343. Use library-go verify package which was created since both CVO and the oc client use them.
jottofar
added a commit
to jottofar/oc
that referenced
this pull request
Sep 15, 2020
Modify to use proper k8s encoding/decoding and other minor cleanup. Reference open comments from openshift#343. Use library-go verify package which was created since both CVO and the oc client use them.
jottofar
added a commit
to jottofar/oc
that referenced
this pull request
Sep 15, 2020
Modify to use proper k8s encoding/decoding and other minor cleanup. Reference open comments from openshift#343. Use library-go verify package which was created since both CVO and the oc client use them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements enhancement openshift/enhancements#283
Local package pkg/verify was reused with minor changes from CVO so there is PR openshift/library-go#725 to move package verify into library-go.