NO-JIRA: Rename ImagePolicy nested types to prevent PKI collision #2626
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
WalkthroughReplaces the previous Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
openshift-ci-robot
added
the
jira/valid-reference
|
@sanchezl: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Hello @sanchezl! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/M
sanchezl
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
This is likely to have a fairly wide impact across the existing image policy implementation when it merges, are the appropriate PRs being lined up to rename this across OCP? |
|
@sanchezl Reached out to me yesterday with the associated PRs but I haven't yet gotten around to taking a look at them: |
The original author expects this to only be used in MCO. I confirmed with a search that that appears to be the case. Given that I did not change the actual field names, the only impact I found was a one line mention of a specific type in the MCO. P.S. The linter insisted that I change some field to/from pointer and that I add tweak some uses of 'omitzero/omitempty`. I will need to propagate those changes to be sure that the impact has remained small. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
config/v1alpha1/types_cluster_image_policy.go (1)
51-55: Clarify whetherspec.policyis truly required now that it’s*ImageScopeVerificationPolicywithomitempty.The field is still documented/annotated as
+required, but the Go type is a pointer and the JSON tag isomitempty. Please confirm whether:
- the API intends
policyto remain required (in which caseomitemptymay be misleading), or- you actually want it optional (and should drop
+required/ update the comment and validation accordingly).Also ensure any consumers of
ClusterImagePolicySpec.Policynow handle anilpointer correctly.config/v1/types_image_policy.go (1)
51-55: Reconcilepolicybeing marked required with its new pointer/omitemptyrepresentation.
ImagePolicySpec.Policyis now*ImageScopeVerificationPolicywithjson:"policy,omitempty"but is still documented and annotated as+required. For this level‑1 stable API, it’s worth being explicit about intent:
- If
policymust always be present, consider droppingomitempty(and/or not using a pointer) to avoid implying optionality.- If you want to allow omitting
policy, then the+requiredmarker and “required field” wording should be adjusted, and CRD/OpenAPI validation regenerated accordingly.Also check that all in‑tree and downstream consumers now nil‑check
spec.Policywhere appropriate.config/v1alpha1/types_image_policy.go (1)
50-54: Double‑checkspec.policyrequiredness vs pointer andomitemptyhere as well.As with the v1 type,
ImagePolicySpec.Policyis now a*ImageScopeVerificationPolicywithomitemptybut still carries a+requiredmarker and “policy contains …” wording that reads as required.Even though this is alpha (compat level 4), it would be good to:
- either keep it truly required and consider dropping
omitempty, or- explicitly make it optional and update annotations/comments plus validation to match.
Clients also now need to guard for
nilspec.Policywhen reading alpha objects.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
config/v1/types_cluster_image_policy.go(1 hunks)config/v1/types_image_policy.go(6 hunks)config/v1/zz_generated.deepcopy.go(9 hunks)config/v1alpha1/types_cluster_image_policy.go(1 hunks)config/v1alpha1/types_image_policy.go(6 hunks)config/v1alpha1/zz_generated.deepcopy.go(9 hunks)openapi/generated_openapi/zz_generated.openapi.go(21 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- config/v1/types_cluster_image_policy.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
config/v1/types_image_policy.goconfig/v1alpha1/types_image_policy.goconfig/v1alpha1/types_cluster_image_policy.goopenapi/generated_openapi/zz_generated.openapi.goconfig/v1/zz_generated.deepcopy.goconfig/v1alpha1/zz_generated.deepcopy.go
🧬 Code graph analysis (4)
config/v1alpha1/types_image_policy.go (1)
config/v1alpha1/zz_generated.swagger_doc_generated.go (4)
ImageScopeVerificationPolicy(295-297)PublicKeyVerificationPolicyRootOfTrust(365-367)FulcioCAWithRekorVerificationPolicyRootOfTrust(247-249)CertificateChainVerificationPolicyRootOfTrust(236-238)
config/v1alpha1/types_cluster_image_policy.go (2)
config/v1alpha1/types_image_policy.go (1)
ImageScopeVerificationPolicy(63-70)config/v1alpha1/zz_generated.swagger_doc_generated.go (1)
ImageScopeVerificationPolicy(295-297)
config/v1/zz_generated.deepcopy.go (3)
config/v1/types_image_policy.go (6)
CertificateChainVerificationPolicyRootOfTrust(176-198)PKICertificateSubject(203-217)ImageScopeVerificationPolicy(64-73)FulcioCAWithRekorVerificationPolicyRootOfTrust(136-154)PolicyIdentity(223-241)PublicKeyVerificationPolicyRootOfTrust(117-133)config/v1alpha1/zz_generated.swagger_doc_generated.go (6)
CertificateChainVerificationPolicyRootOfTrust(236-238)PKICertificateSubject(305-307)ImageScopeVerificationPolicy(295-297)FulcioCAWithRekorVerificationPolicyRootOfTrust(247-249)PolicyIdentity(326-328)PublicKeyVerificationPolicyRootOfTrust(365-367)config/v1/zz_generated.swagger_doc_generated.go (6)
CertificateChainVerificationPolicyRootOfTrust(1224-1226)PKICertificateSubject(1294-1296)ImageScopeVerificationPolicy(1284-1286)FulcioCAWithRekorVerificationPolicyRootOfTrust(1235-1237)PolicyIdentity(1315-1317)PublicKeyVerificationPolicyRootOfTrust(1354-1356)
config/v1alpha1/zz_generated.deepcopy.go (2)
config/v1alpha1/types_image_policy.go (5)
CertificateChainVerificationPolicyRootOfTrust(155-175)PKICertificateSubject(180-194)ImageScopeVerificationPolicy(63-70)FulcioCAWithRekorVerificationPolicyRootOfTrust(124-138)PublicKeyVerificationPolicyRootOfTrust(110-121)config/v1alpha1/zz_generated.swagger_doc_generated.go (5)
CertificateChainVerificationPolicyRootOfTrust(236-238)PKICertificateSubject(305-307)ImageScopeVerificationPolicy(295-297)FulcioCAWithRekorVerificationPolicyRootOfTrust(247-249)PublicKeyVerificationPolicyRootOfTrust(365-367)
🔇 Additional comments (12)
config/v1/types_image_policy.go (1)
63-73: ImageScopeVerificationPolicy and root‑of‑trust type renames look consistent.Renaming
Policy→ImageScopeVerificationPolicyand the root‑of‑trust structs (PublicKey*,FulcioCAWithRekor*,PKI→CertificateChainVerificationPolicyRootOfTrust) is internally consistent here and in the union (PolicyRootOfTrust). JSON field names and discriminator values are preserved, so on‑the‑wire compatibility is maintained while avoiding future type name collisions.Also applies to: 92-104, 116-155, 175-198
config/v1alpha1/types_image_policy.go (1)
62-97: Renamed verification policy and root‑of‑trust types are coherent with the v1 surface.The
ImageScopeVerificationPolicyrename and the new*VerificationPolicyRootOfTruststructs, along with updatingPolicyRootOfTrustto use pointers to these types, align v1alpha1 with the v1 API while avoiding the planned PKI name collision. Union annotations and JSON field names (publicKey,fulcioCAWithRekor,pki) remain consistent.Also applies to: 109-121, 123-138, 154-175
config/v1/zz_generated.deepcopy.go (2)
927-952: DeepCopy implementations for renamed root‑of‑trust types look correct.The generated DeepCopy functions for
CertificateChainVerificationPolicyRootOfTrust,FulcioCAWithRekorVerificationPolicyRootOfTrust,PublicKeyVerificationPolicyRootOfTrust, and the updatedPolicyRootOfTrustall correctly:
- copy scalar fields by value, and
- allocate and copy the underlying
[]byteslices and nested structs where present.This matches the struct definitions in
config/v1/types_image_policy.goand avoids shared backing storage.Also applies to: 2375-2399, 5601-5624, 5278-5287
1121-1132: PointerPolicyfields are deep‑copied safely for Image/ClusterImagePolicy specs.
ClusterImagePolicySpec.DeepCopyIntoandImagePolicySpec.DeepCopyIntonow correctly handlenilvs non‑nilPolicyby allocating a newImageScopeVerificationPolicyand delegating to its DeepCopy. TheImageScopeVerificationPolicyDeepCopy in turn deep‑copiesRootOfTrustandSignedIdentity, so no shallow copies remain.Also applies to: 3135-3147, 3183-3203
openapi/generated_openapi/zz_generated.openapi.go (1)
183-183: Generated code changes are consistent with the type renames.This auto-generated file correctly reflects the type renames across both v1 and v1alpha1 API versions:
PKI→CertificateChainVerificationPolicyRootOfTrustFulcioCAWithRekor→FulcioCAWithRekorVerificationPolicyRootOfTrustPublicKey→PublicKeyVerificationPolicyRootOfTrustPolicy→ImageScopeVerificationPolicyAll schema definitions, references, and dependencies are updated uniformly. The
PKICertificateSubjecttype is correctly preserved as it was outside the scope of this rename.Also applies to: 245-245, 279-279, 377-377, 431-431, 442-442, 448-448, 462-462, 10095-10130, 10471-10483, 12796-12803, 14345-14357, 14396-14423, 18391-18431, 18984-18991, 21531-21567, 21696-21708, 21976-21983, 22184-22196, 22235-22263, 22729-22767, 22770-22777
config/v1alpha1/zz_generated.deepcopy.go (7)
195-220: LGTM: Correct deepcopy for CertificateChainVerificationPolicyRootOfTrust.The generated deepcopy implementation properly handles the byte slice fields and struct field for the renamed type.
291-295: LGTM: Correct pointer handling for ClusterImagePolicySpec.Policy.The generated code properly handles the Policy field as a pointer to ImageScopeVerificationPolicy, with appropriate nil checking and deep copying.
464-488: LGTM: Correct deepcopy for FulcioCAWithRekorVerificationPolicyRootOfTrust.The generated deepcopy implementation properly handles byte slices and the FulcioSubject field for the renamed type.
585-589: LGTM: Consistent pointer handling for ImagePolicySpec.Policy.The generated code properly handles the Policy field with the same correct pattern used in ClusterImagePolicySpec.
626-642: LGTM: Correct deepcopy for ImageScopeVerificationPolicy.The generated deepcopy implementation properly calls DeepCopyInto on the RootOfTrust and SignedIdentity nested fields.
907-924: LGTM: Correct type references in PolicyRootOfTrust deepcopy.The generated code properly references all three renamed root-of-trust types (PublicKeyVerificationPolicyRootOfTrust, FulcioCAWithRekorVerificationPolicyRootOfTrust, CertificateChainVerificationPolicyRootOfTrust) with correct deepcopy logic.
938-961: LGTM: Correct deepcopy for PublicKeyVerificationPolicyRootOfTrust.The generated deepcopy implementation properly handles both byte slice fields (KeyData and RekorKeyData) for the renamed type.
This change resulted in no changes to the client-go generated code. |
| // images not matching the verification policy will be treated. | ||
| // +required | ||
| Policy Policy `json:"policy"` | ||
| Policy *ImageScopeVerificationPolicy `json:"policy,omitempty"` |
There was a problem hiding this comment.
Please don't change this to a pointer/change the omitempty. The field has shipped and we shouldn't change the structure now as this could impact folks who've built tooling against these types
There was a problem hiding this comment.
Got it. How do I get past the linter?
There was a problem hiding this comment.
/override at the discretion of the API approvers
sanchezl
force-pushed
the
cluster-image-policy-rename
branch
from
756c352 to
78d4b6a
Compare
|
/override ci/prow/lint The linting errors cannot be fixed on these pre-existing APIs without breaking folks. So will override. Looking at the PRs, you've got client-go and MCO to update, are those the only two? Have we tested those sufficiently? Looks like the MCO one has a bunch of red on it |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/lint DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
QiWang19
left a comment
QiWang19
left a comment
There was a problem hiding this comment.
I suggested renaming to better reflect the specific intent. Hope it works to avoid naming collisions.
sanchezl
force-pushed
the
cluster-image-policy-rename
branch
from
78d4b6a to
65b693c
Compare
sanchezl
force-pushed
the
cluster-image-policy-rename
branch
from
65b693c to
adfa0f5
Compare
openshift-ci
bot
removed
the
size/M
openshift-ci
bot
added
the
size/L
Thank you, I had to rebase with the naming preferred by @QiWang19 (the engineer who introduced these types).
I confirmed with @QiWang19 that there should not be any others.
There were some updated commits I needed to push. It should look much better now. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
openapi/generated_openapi/zz_generated.openapi.go (2)
14194-14229: LGTM: Root-of-trust type definitions are structurally sound.The new
ImagePolicy*RootOfTrusttype schemas are properly defined with correct field types, required fields, and dependency references for both v1 and v1alpha1.Optional: Documentation string consistency.
Minor documentation inconsistencies exist between v1 and v1alpha1 (e.g., Line 14203 "is a required field contains" vs Line 22043 "contains"), and Line 22167 has inconsistent capitalization ("KeyData" should be "keyData"). Since this is generated code, these likely originate from source type definitions and should be addressed there if cleanup is desired.
Also applies to: 14282-14345, 22034-22069, 22121-22184
14426-14453: LGTM: ImageSigstoreVerificationPolicy schemas defined correctly.The new verification policy type is properly defined with required
rootOfTrustand optionalsignedIdentityfields, correctly referencingPolicyRootOfTrustandPolicyIdentitytypes for both API versions.Optional: Consider v1/v1alpha1 Default value alignment.
v1alpha1's
signedIdentity(Line 22282) has aDefaultvalue while v1's (Line 14441) does not. This may be intentional API evolution, but if consistent behavior is desired across versions, verify whether this difference is intended.Also applies to: 22265-22293
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
config/v1/zz_generated.deepcopy.go(4 hunks)config/v1/zz_generated.swagger_doc_generated.go(3 hunks)config/v1alpha1/zz_generated.deepcopy.go(4 hunks)config/v1alpha1/zz_generated.swagger_doc_generated.go(3 hunks)openapi/generated_openapi/zz_generated.openapi.go(16 hunks)openapi/openapi.json(13 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
config/v1alpha1/zz_generated.deepcopy.goopenapi/openapi.jsonconfig/v1/zz_generated.swagger_doc_generated.goopenapi/generated_openapi/zz_generated.openapi.goconfig/v1alpha1/zz_generated.swagger_doc_generated.goconfig/v1/zz_generated.deepcopy.go
🧬 Code graph analysis (1)
config/v1/zz_generated.deepcopy.go (1)
config/v1/types_image_policy.go (6)
ImagePolicyFulcioCAWithRekorRootOfTrust(136-154)ImagePolicyPKIRootOfTrust(176-198)PKICertificateSubject(203-217)ImagePolicyPublicKeyRootOfTrust(117-133)ImageSigstoreVerificationPolicy(64-73)PolicyIdentity(223-241)
🔇 Additional comments (16)
openapi/openapi.json (3)
5207-5213: Type renames applied correctly for v1 API.The renames from
PolicytoImageSigstoreVerificationPolicyand the root-of-trust types (ImagePolicyFulcioCAWithRekorRootOfTrust,ImagePolicyPKIRootOfTrust,ImagePolicyPublicKeyRootOfTrust) are applied consistently throughout the v1 schema definitions. All$refpointers correctly reference the v1 namespace.Also applies to: 7433-7592, 9893-9912
11794-11800: Type renames applied correctly for v1alpha1 API.The v1alpha1 schema mirrors the v1 changes with proper namespace prefixing. All renamed types and references are consistent between API versions.
Also applies to: 12012-12171, 12439-12458
7433-7458: Verify downstream consumers handle the breaking type rename.The type rename from
FulcioCAWithRekortoImagePolicyFulcioCAWithRekorRootOfTrust(and similar renames) is a breaking change for any client code directly referencing these types. MCO is the only known consumer with a corresponding PR (openshift/machine-config-operator#5500, OPEN) that bumps openshift/client-go (openshift/client-go#354, OPEN).Also applies to: 12012-12037
config/v1/zz_generated.deepcopy.go (5)
3043-3068: DeepCopy forImagePolicyFulcioCAWithRekorRootOfTrustcorrectly handles slice fields
FulcioCADataandRekorKeyDataslices are deep-copied andFulcioSubject(value type) is copied by value, which matches the struct definition and avoids aliasing.
3103-3128: DeepCopy forImagePolicyPKIRootOfTrustmatches BYO‑PKI shapeBoth certificate bundle slices are deep-copied and
PKICertificateSubject(value type) is copied by value; this is consistent with the struct intypes_image_policy.goand avoids shared slice backing arrays.
3131-3154: DeepCopy forImagePolicyPublicKeyRootOfTrustcorrectly copies key material
KeyDataand optionalRekorKeyDataare deep-copied as new byte slices; no shared state is retained between copies.
3201-3221: DeepCopy forImageSigstoreVerificationPolicyrespects nested union and pointer
RootOfTrustis deep-copied viaDeepCopyInto, andSignedIdentityis cloned with proper allocation andDeepCopyInto. This preserves the discriminated union semantics without pointer aliasing.
5296-5314:PolicyRootOfTrustnow deep-copies renamed root‑of‑trust typesPointer fields
PublicKey,FulcioCAWithRekor, andPKIare retyped toImagePolicy*RootOfTrust, allocated, and deep-copied, which aligns with the type rename and maintains correct deep-copy behavior for the union members.config/v1alpha1/zz_generated.swagger_doc_generated.go (4)
240-249: Swagger docs forImagePolicyFulcioCAWithRekorRootOfTrustalign with type definitionField descriptions for
fulcioCAData,rekorKeyData, andfulcioSubjectmatch the struct semantics (Fulcio CA, Rekor key, and subject) and use the updated type name, so generated API docs remain accurate.
260-269: Swagger docs forImagePolicyPKIRootOfTrustcorrectly describe BYO‑PKI fieldsThe new map documents
caRootsData,caIntermediatesData, andpkiCertificateSubjectconsistently with the PKI root‑of‑trust struct, preserving user‑facing behavior while reflecting the renamed type.
271-279: Swagger docs forImagePolicyPublicKeyRootOfTrustmatch sigstore key usageDescriptions for
keyDataand optionalrekorKeyDataaccurately reflect the public key and Rekor key fields of the renamed root‑of‑trust type; no API semantics change.
299-307:ImageSigstoreVerificationPolicyswagger doc matches verification policy structSummary and field docs for
rootOfTrustandsignedIdentityare consistent with the struct definition and the discriminator-based identity matching, and correctly use the new policy type name.openapi/generated_openapi/zz_generated.openapi.go (3)
271-280: LGTM: Map entries added consistently.The new type entries are properly added to the OpenAPI definitions map for both v1 and v1alpha1, following the established naming convention.
Also applies to: 443-449
10435-10448: LGTM: Policy references updated consistently.The policy field references have been correctly updated to use
ImageSigstoreVerificationPolicyin bothClusterImagePolicySpecandImagePolicySpecfor v1 and v1alpha1, with matching dependency lists.Also applies to: 14375-14388, 21660-21673, 22214-22227
18421-18439: LGTM: PolicyRootOfTrust references updated correctly.All references within
PolicyRootOfTrusthave been updated to use the newImagePolicy*RootOfTrusttype names, and dependency lists have been updated accordingly for both v1 and v1alpha1.Also applies to: 18455-18460, 22759-22777, 22793-22798
config/v1alpha1/zz_generated.deepcopy.go (1)
486-943: LGTM! Generated deepcopy code correctly implements the type renames.The auto-generated deepcopy methods for the new
ImagePolicy*RootOfTrusttypes andImageSigstoreVerificationPolicyare correct. The implementations properly handle:
- Deep copying of byte slices with appropriate nil checks and allocations
- Correct delegation to nested struct deepcopy methods
- Updated type references in
PolicyRootOfTrust.DeepCopyIntoto use the renamed typesThe changes align with the PR objective to rename nested types and avoid PKI collision.
| var map_ImagePolicyFulcioCAWithRekorRootOfTrust = map[string]string{ | ||
| "": "ImagePolicyFulcioCAWithRekorRootOfTrust defines the root of trust based on the Fulcio certificate and the Rekor public key.", | ||
| "fulcioCAData": "fulcioCAData is a required field contains inline base64-encoded data for the PEM format fulcio CA. fulcioCAData must be at most 8192 characters. ", | ||
| "rekorKeyData": "rekorKeyData is a required field contains inline base64-encoded data for the PEM format from the Rekor public key. rekorKeyData must be at most 8192 characters. ", | ||
| "fulcioSubject": "fulcioSubject is a required field specifies OIDC issuer and the email of the Fulcio authentication configuration.", | ||
| } | ||
|
|
||
| func (ImagePolicyFulcioCAWithRekorRootOfTrust) SwaggerDoc() map[string]string { | ||
| return map_ImagePolicyFulcioCAWithRekorRootOfTrust | ||
| } |
There was a problem hiding this comment.
Root-of-trust type renames look correct; consider tightening doc phrasing
The three new SwaggerDoc maps for ImagePolicyFulcioCAWithRekorRootOfTrust, ImagePolicyPKIRootOfTrust, and ImagePolicyPublicKeyRootOfTrust line up with the PR’s intent (distinct ImagePolicy-scoped root-of-trust variants with size limits and subject constraints), and the field keys look consistent with the JSON field names.
If you touch these comments again, you might want to fix the slightly awkward wording in the user-facing descriptions, e.g.:
- “
fulcioCADatais a required field contains …” → “is a required field that contains …” - “
rekorKeyDatais an optional field contains …” → “is an optional field that contains …” - “
fulcioSubjectis a required field specifies …” → “is a required field that specifies …”
Same pattern applies to keyData/rekorKeyData in ImagePolicyPublicKeyRootOfTrust.
This is non-blocking, since it’s only doc text.
Also applies to: 1249-1258, 1260-1268
🤖 Prompt for AI Agents
In config/v1/zz_generated.swagger_doc_generated.go around lines 1228-1237 (and
similarly at 1249-1258, 1260-1268), the SwaggerDoc strings use awkward phrasing
like "is a required field contains" / "is an optional field contains" / "is a
required field specifies"; update those user-facing descriptions to include
"that" after "field" so they read e.g. "is a required field that contains ..."
or "is an optional field that contains ..." and "is a required field that
specifies ...", and make the same wording fix for the corresponding
keyData/rekorKeyData/fulcioSubject entries in the other two maps.
| var map_ImageSigstoreVerificationPolicy = map[string]string{ | ||
| "": "ImageSigstoreVerificationPolicy defines the verification policy for the items in the scopes list.", | ||
| "rootOfTrust": "rootOfTrust is a required field that defines the root of trust for verifying image signatures during retrieval. This allows image consumers to specify policyType and corresponding configuration of the policy, matching how the policy was generated.", | ||
| "signedIdentity": "signedIdentity is an optional field specifies what image identity the signature claims about the image. This is useful when the image identity in the signature differs from the original image spec, such as when mirror registry is configured for the image scope, the signature from the mirror registry contains the image identity of the mirror instead of the original scope. The required matchPolicy field specifies the approach used in the verification process to verify the identity in the signature and the actual image identity, the default matchPolicy is \"MatchRepoDigestOrExact\".", | ||
| } | ||
|
|
||
| func (PKI) SwaggerDoc() map[string]string { | ||
| return map_PKI | ||
| func (ImageSigstoreVerificationPolicy) SwaggerDoc() map[string]string { | ||
| return map_ImageSigstoreVerificationPolicy | ||
| } |
There was a problem hiding this comment.
ImageSigstoreVerificationPolicy mapping matches model; minor doc polish possible
map_ImageSigstoreVerificationPolicy and its SwaggerDoc correctly describe the new policy type and reference rootOfTrust/signedIdentity in a way that’s consistent with the surrounding PolicyRootOfTrust and PolicyIdentity docs.
As with the root-of-trust types, the description for signedIdentity (“is an optional field specifies …”) could be tightened to “is an optional field that specifies …” if you’re revisiting the comments, but this is purely cosmetic.
🤖 Prompt for AI Agents
In config/v1/zz_generated.swagger_doc_generated.go around lines 1288 to 1296,
the documentation string for the "signedIdentity" map entry has a minor
grammatical issue; change "is an optional field specifies what image
identity..." to "is an optional field that specifies what image identity..." so
the sentence is grammatically correct while keeping the rest of the wording
unchanged.
|
/approve This is good by me, @QiWang19 can you LGTM if you're happy |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/lint DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@sanchezl: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-ci
bot
added
the
approved
|
I just realized, apart from MCO, the ClusterImagePolicy and ImagePolicy API struct is also used in the origin test https://github.com/openshift/origin/blob/18eb1d61068413a12aba5867715f5036189b8a8c/test/extended/imagepolicy/imagepolicy.go#L364, we will also need a follow-up PR there. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed, QiWang19 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5 participants
Rename some types in the config package to prevent name collisions.
I was specifically interested in the
PKItype used in theImagePolicySpecbecause it conflicts with a new top level type I want to introduce. I also renamed the other types in theImagePolicySpecdiscriminated union for consistency. Additionally, I renamed thePolicytype. The machine-config-operator is the only know client of these types, and would require a single one-line update when it bumps to an openshift/client-go version containing these changes.I did not do an extensive search amongst the entire package and focused only on these few cases.