NO-ISSUE: Synchronize From Upstream Repositories #601
Merged
Conversation
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
… of CI (#2411) * Add a Makefile target and start running the API diff linter as part of CI. * Update hack/api-lint-diff/run.sh Co-authored-by: Todd Short <tmshort@users.noreply.github.com> * Run for pull requests only * Update .github/workflows/api-diff-lint.yaml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/workflows/api-diff-lint.yaml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update hack/api-lint-diff/run.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Makefile Co-authored-by: Predrag Knezevic <pedjak@gmail.com> * Apply suggestion from to improve branch check * Apply suggestion from @camilamacedo86 By Copilot: When sourcing .bingo/variables.env, the file contains GOBIN=${GOBIN:=$(go env GOBIN)} which requires the go command to be available. While this works in CI after the setup-go step, the script might fail if run locally without Go in PATH. Consider adding a check that Go is available before sourcing, or handle the source operation with error checking using source .bingo/variables.env 2>/dev/null || true and validate the variable afterward. * Update .github/workflows/api-diff-lint.yaml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Todd Short <tmshort@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Predrag Knezevic <pedjak@gmail.com>
openshift-bot
added
tide/merge-method-merge
openshift-ci-robot
added
the
jira/valid-reference
|
@openshift-bot: 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. |
Contributor
|
@openshift-bot: GitHub didn't allow me to request PR reviews from the following users: openshift/openshift-team-operator-framework. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot 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 |
1 similar comment
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: openshift-bot 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 |
…viceAccount for revision operations (#2429) * (feat): [Boxcutter] Use ClusterExtension ServiceAccount for revision operations Implement serviceAccount-scoped token-based authentication for ClusterExtensionRevision controller using annotation-based configuration. - Add RevisionEngineFactory with CreateRevisionEngine(ctx, rev) interface - Read ServiceAccount from annotations (no ClusterExtension dependency) - Token-based auth using TokenInjectingRoundTripper - ServiceAccount name and namespace in annotations for observability - TrackingCache uses global client for caching/cleanup - Comprehensive error path tests ClusterExtensionRevision can exist independently. Easy mode impersonation deferred until API is finalized. Assisted-by: Cursor * (doc) Add godoc comments to label constants Adds documentation comments to all label/annotation constants explaining: - What each constant represents - Where they are applied (labels vs annotations) - ServiceAccount constants document their relationship to ClusterExtension spec Addresses code review feedback for improved maintainability. * (fix) Add ClusterExtensionRevision permissions to upgrade test RBAC The upgrade test ServiceAccount needs permissions to manage ClusterExtensionRevisions when BoxcutterRuntime is enabled. Without these permissions, the upgraded controller cannot create or update ClusterExtensionRevision resources, causing the ClusterExtension to fail reconciliation after upgrade. * review changes * (fix): e2e: add bind/escalate verbs for Boxcutter Server-Side Apply Add `bind` and `escalate` RBAC verbs to e2e test ServiceAccount to support Boxcutter applier's use of Kubernetes Server-Side Apply (SSA). Experimental e2e tests fail when Boxcutter uses ServiceAccount-scoped clients to apply bundle RBAC resources (ClusterRoles and ClusterRoleBindings): ``` clusterrolebindings.rbac.authorization.k8s.io is forbidden: User "system:serviceaccount:olmv1-e2e:olm-sa" cannot bind ClusterRole: RBAC: attempting to grant RBAC permissions not currently held ``` - Uses helm.sh/helm/v3 library - Applies resources with traditional CREATE/UPDATE operations - Kubernetes RBAC allows ClusterRoleBinding creation when the ServiceAccount already has all the permissions being granted (permission matching) - **Works WITHOUT `bind`/`escalate` verbs** ✅ - Uses pkg.package-operator.run/boxcutter machinery - Applies resources with **Server-Side Apply (SSA)** (`client.Apply`) - SSA enforces field-level ownership and **stricter RBAC enforcement** - Kubernetes API server **requires explicit `bind` verb** for ClusterRoleBindings - Permission matching fallback does NOT work reliably with SSA - **REQUIRES `bind`/`escalate` verbs** ❌ Validated by running actual tests: **Test 1: Main branch standard-e2e (Helm, NO bind/escalate)** ```bash make test-e2e ``` Result: ✅ PASS (21 scenarios passed) **Test 2: PR branch experimental-e2e (Boxcutter, NO bind/escalate)** ```bash make test-experimental-e2e ``` Result: ❌ FAIL (cannot bind ClusterRole error) **Test 3: PR branch experimental-e2e (Boxcutter, WITH bind/escalate)** Result: ✅ PASS (all tests pass) Add `bind` and `escalate` verbs to the e2e test RBAC template: ```yaml - apiGroups: ["rbac.authorization.k8s.io"] resources: [clusterroles, roles, clusterrolebindings, rolebindings] verbs: [ update, create, list, watch, get, delete, patch, bind, escalate ] ``` These verbs allow the ServiceAccount to: - `bind`: Create ClusterRoleBindings that reference roles with permissions the ServiceAccount doesn't have - `escalate`: Create ClusterRoles with permissions the ServiceAccount doesn't have This is the documented requirement in `docs/concepts/permission-model.md` for extension installers and aligns with Kubernetes RBAC best practices. 1. **Required for SSA**: Server-Side Apply has stricter RBAC enforcement 2. **Documented requirement**: OLMv1 docs specify bind/escalate as proper approach 3. **Industry best practice**: Operator installers should have these verbs 4. **Supports all operators**: Not just test-operator with matching permissions 5. **Maintains SSA benefits**: Field ownership, conflict resolution, GitOps support - Kubernetes RBAC: https://kubernetes.io/docs/reference/access-authn-authz/rbac/#privilege-escalation-prevention-and-bootstrapping - OLMv1 Permission Model: docs/concepts/permission-model.md - Boxcutter machinery: pkg.package-operator.run/boxcutter/machinery (uses client.Apply) - Testing evidence: FINAL_TESTED_ANSWER.md, SERVER_SIDE_APPLY_ANSWER.md Tested-by: Actual e2e test runs comparing Helm vs Boxcutter behavior Signed-off-by: Camila <camil@example.com> * Split rbac phase into two Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> --------- Signed-off-by: Camila <camil@example.com> Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
Updating `ClusterExtension` with duplicate entry under `.status.activeRevisions` fails. Thus, we repopulate it from the installed and rolling out revisions.
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
810449b to
0570027
Compare
Bumps [github.com/sigstore/fulcio](https://github.com/sigstore/fulcio) from 1.7.1 to 1.8.5. - [Release notes](https://github.com/sigstore/fulcio/releases) - [Changelog](https://github.com/sigstore/fulcio/blob/main/CHANGELOG.md) - [Commits](sigstore/fulcio@v1.7.1...v1.8.5) --- updated-dependencies: - dependency-name: github.com/sigstore/fulcio dependency-version: 1.8.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Change Apply() to return only error instead of (bool, string, error), removing status interpretation logic from the applier. ClusterExtensionRevision conditions are already mirrored to ClusterExtension. - Change ApplyBundleWithBoxcutter to accept a function instead of an interface, making unit tests simpler by allowing inline function mocks Co-authored-by: Claude <noreply@anthropic.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
0570027 to
5ad3ff8
Compare
Member
|
/verified bypass |
openshift-ci-robot
added
the
verified
|
@jianzhangbjz: The 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. |
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
5ad3ff8 to
a94bd3e
Compare
openshift-ci-robot
removed
the
verified
|
@openshift-bot: 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. |
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
Bumps [pymdown-extensions](https://github.com/facelessuser/pymdown-extensions) from 10.19.1 to 10.20. - [Release notes](https://github.com/facelessuser/pymdown-extensions/releases) - [Commits](facelessuser/pymdown-extensions@10.19.1...10.20) --- updated-dependencies: - dependency-name: pymdown-extensions dependency-version: '10.20' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
a94bd3e to
cf987e2
Compare
Signed-off-by: Todd Short <todd.short@me.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Expose docker-registry to e2e test code by creating Openshift route
Merge of openshift@9fbe333 enabled proper exposure of docker-registry to upstream e2e tests. Thus, we are now able to run downstream the tests tagged with `@catalogd-update`
openshift-bot
force-pushed
the
synchronize-upstream
branch
from
cf987e2 to
164761e
Compare
Contributor
|
/verified by @Xia-Zhao-rh |
openshift-ci-robot
added
the
verified
|
@Xia-Zhao-rh: This PR has been marked as verified by 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. |
Member
|
/lgtm |
Contributor
|
@openshift-bot: all tests passed! 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 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.
The downstream repository has been updated with the following following upstream commits:
The
vendor/directory has been updated and the following commits were carried:@catalogd-updateThis pull request is expected to merge without any human intervention. If tests are failing here, changes must land upstream to fix any issues so that future downstreaming efforts succeed.
/cc @openshift/openshift-team-operator-framework