cert-manager: build crd-model-gen image locally in update.sh#4751
Open
YuqiGuo105 wants to merge 1 commit into
Open
cert-manager: build crd-model-gen image locally in update.sh#4751YuqiGuo105 wants to merge 1 commit into
YuqiGuo105 wants to merge 1 commit into
Conversation
k8s-ci-robot
added
do-not-merge/invalid-commit-message
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: YuqiGuo105 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/L
YuqiGuo105
force-pushed
the
feat/managed-fields-include-toggle
branch
from
cefaf63 to
ea16f5f
Compare
k8s-ci-robot
removed
the
do-not-merge/invalid-commit-message
Contributor
|
I don't think this change includes what you are thinking, it doesn't remove an alpha files. |
The default crd-model-gen image referenced by update.sh lives on docker.pkg.github.com, which requires GitHub authentication. Anonymous docker pulls fail with "Access is denied", so contributors who try to follow the README cannot run update.sh out of the box (reported by multiple users in the comments on issue 3489). Detect the missing image at runtime and build it locally from the Dockerfile shipped in client-java-contrib/, defaulting to local/crd-model-gen:latest. IMAGE_NAME and IMAGE_TAG can still be set to point at a prebuilt image. Also refresh README.md so the dependency snippet and compatibility table match the current artifact version (27.0.0-SNAPSHOT) and the v1.13.4 CRD URL already used by update.sh.
YuqiGuo105
force-pushed
the
feat/managed-fields-include-toggle
branch
from
ea16f5f to
9ba7c60
Compare
k8s-ci-robot
added
size/S
YuqiGuo105
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Background
Issue #3489 has two distinct problems mixed together. The original report (V1 cert-manager models missing from the published jar) was already addressed on
masterlong ago:client-java-contrib/cert-manager/src/main/java/io/cert/manager/models/now contains 109 V1-only types, with noV1alpha*/V1beta*classes left. So that part doesn't need a code change.This PR addresses the other problem raised in the comments on #3489: contributors who try to follow the README and run
./update.share blocked at the first step because docker pulls fail withAccess is denied. The default image referenced byupdate.sh,lives on a registry that requires GitHub authentication, so anonymous pulls do not work. This was reported by @vaidikcode and @AyushSinha-6409 in the issue thread.
Changes
client-java-contrib/cert-manager/update.sh: detect the missing image at runtime and build it locally from theDockerfilealready shipped inclient-java-contrib/, defaulting tolocal/crd-model-gen:latest.IMAGE_NAMEandIMAGE_TAGcan still be overridden to use a prebuilt image.client-java-contrib/cert-manager/README.md: refresh the dependency snippet and compatibility table so they match the current artifact version (27.0.0-SNAPSHOT) and the v1.13.4 CRD URL already used byupdate.sh. No regeneration of model classes is included; the existing V1 classes onmasterare unchanged.Verification
Running
./update.shfrom a clean checkout (with no prebuilt image present) now buildslocal/crd-model-gen:latestfrom the in-treeDockerfileand proceeds with the generation pipeline without contacting the private registry.Refs: #3489