Skip to content

feat: functions exposed via ocp-route - keda + raw deployers - #3973

Draft
gauron99 wants to merge 2 commits into
knative:mainfrom
gauron99:push-lqomtzmulusl
Draft

feat: functions exposed via ocp-route - keda + raw deployers#3973
gauron99 wants to merge 2 commits into
knative:mainfrom
gauron99:push-lqomtzmulusl

Conversation

@gauron99

Copy link
Copy Markdown
Contributor

Changes

/kind

Fixes #

Release Note


Docs


@knative-prow

knative-prow Bot commented Jul 27, 2026

Copy link
Copy Markdown

@gauron99: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

Details

In response to this:

Changes

/kind

Fixes #

Release Note


Docs


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.

@knative-prow

knative-prow Bot commented Jul 27, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@knative-prow knative-prow Bot added do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. size/XXL 🤖 PR changes 1000+ lines, ignoring generated files. labels Jul 27, 2026
@knative-prow
knative-prow Bot requested review from dsimansk and jrangelramos July 27, 2026 22:27
@knative-prow

knative-prow Bot commented Jul 27, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauron99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow Bot added the approved 🤖 PR has been approved by an approver from all required OWNERS files. label Jul 27, 2026
@gauron99
gauron99 requested review from Copilot and removed request for dsimansk and jrangelramos July 27, 2026 22:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds configurable external exposure for the raw and keda deployers via a new deploy.expose field / --expose flag, with OpenShift Route support and improved URL reporting across deploy, list, and describe flows.

Changes:

  • Introduces deploy.expose (route|none|"") with validation, CLI flag + completion, schema/docs updates, and new CLI error type for invalid values.
  • Implements OpenShift Route lifecycle management for the raw deployer (Route creation/removal + admission wait + recording hostname on the Service for later reads).
  • Updates the keda deployer to manage its own Route to the interceptor (and keeps tests/e2e/integration helpers deterministic by opting out with expose:none).

Reviewed changes

Copilot reviewed 37 out of 38 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
schema/func_yaml-schema.json Adds deploy.expose to func.yaml JSON schema.
pkg/remover/testing/integration_test_helper.go Forces integration test deploys to be cluster-local (Expose: "none").
pkg/pipelines/tekton/pipelines_provider.go Aligns deploy status messaging (neutral “at URL”).
pkg/mock/deployer.go Sets a successful mock deploy status (now always Deployed).
pkg/lister/testing/integration_test_helper.go Forces integration test deploys to be cluster-local (Expose: "none").
pkg/knative/deployer.go Adjusts verbose message wording (removes “exposed”).
pkg/keda/route.go Adds OpenShift Route generation/ensure/remove helpers for the keda interceptor.
pkg/keda/route_test.go Unit tests for keda interceptor Route helpers and URL selection.
pkg/keda/remover.go Removes keda interceptor Route during removal on OpenShift.
pkg/keda/remover_test.go Tests Route-removal ordering and nil-dynamic-client behavior.
pkg/keda/lister.go Prefers external Route URL when present (by deterministic Route name).
pkg/keda/describer.go Reports URLs including external Route (when admitted) + internal bridge URLs.
pkg/keda/deployer.go Adds deploy.expose handling and Route lifecycle for keda (interceptor Route).
pkg/keda/deployer_test.go Tests interceptor namespace resolution and bridge ExternalName.
pkg/k8s/wait.go Generalizes WaitForServiceRemoved to kubernetes.Interface.
pkg/k8s/route.go Adds OpenShift Route CRUD/admission utilities via dynamic client.
pkg/k8s/route_test.go Unit tests for k8s Route utilities (ensure/remove/admit/read).
pkg/k8s/lister.go Uses Service annotation to surface external HTTPS URL when exposed.
pkg/k8s/describer.go Reports external+internal routes when exposed (via Service annotation).
pkg/k8s/deployer.go Adds exposure resolution (Route ensure/remove + Service annotation) for raw deployer; refactors resource-check messaging.
pkg/k8s/deployer_test.go Adds tests for exposure gating and referenceCheckMessage.
pkg/functions/function.go Adds DeploySpec.Expose and validates it in Function.Validate().
pkg/functions/function_expose.go Adds ValidateExpose + validateExpose (library + validation errors).
pkg/functions/function_expose_unit_test.go Unit tests for expose validation and Function.Validate integration.
pkg/functions/errors.go Adds ErrInvalidExpose.
pkg/functions/client.go Updates deploy success message wording (removes “exposed”).
pkg/functions/client_test.go Tests that Deploy prints success message containing namespace + URL.
pkg/describer/testing/integration_test_helper.go Forces integration test deploys to be cluster-local (Expose: "none").
pkg/deployer/testing/integration_test_helper.go Forces multiple integration test deploys to be cluster-local (Expose: "none").
go.mod Bumps sigs.k8s.io/gateway-api indirect dependency to v1.5.1.
go.sum Updates checksums for the gateway-api bump.
e2e/e2e_trigger_sync_test.go Ensures raw deploys in e2e are cluster-local (--expose none).
e2e/e2e_metadata_test.go Ensures raw deploys in e2e are cluster-local (--expose none).
docs/reference/func_deploy.md Documents new --expose flag.
cmd/errors.go Wraps invalid expose values with a typed CLI error (help text).
cmd/deploy.go Adds --expose flag, validation, completion, and warning when ignored.
cmd/deploy_test.go Adds CLI tests for expose persistence/clearing, invalid values, and ignore warning.
cmd/completion_util.go Adds shell completion for --expose.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/mock/deployer.go
Comment thread cmd/deploy.go Outdated
Comment thread cmd/deploy.go Outdated
Comment thread pkg/k8s/route.go Outdated
Comment thread schema/func_yaml-schema.json

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

pkg/mock/deployer.go:44

  • pkg/mock/NewDeployer always returns Status=Deployed on success, even when redeploying an already-deployed function (f.Deploy.Namespace set). This makes the mock less faithful and can hide update-path behavior (e.g., status messages that differ for Updated). Set Status=Updated when f.Deploy.Namespace is non-empty and no explicit new namespace was requested.
    schema/func_yaml-schema.json:140
  • schema/func_yaml-schema.json is listed as a generated file in AGENTS.md, so it shouldn’t be edited manually. Please regenerate it via the project’s generator (e.g. ./hack/update-codegen.sh / make all) and commit the regenerated output, to avoid hand-edited drift.
    pkg/keda/lister.go:86
  • This creates a new dynamic client inside Lister.get() for every listed function on OpenShift, which can be unnecessarily expensive (config loading/client construction in a tight loop). Consider creating the dynamic client once in List() (when IsOpenShift), reusing it across items, and passing it into get() (or storing it on the Lister).
	if k8s.IsOpenShift() {
		if dynClient, err := k8s.NewDynamicClient(); err == nil {
			routeHost, routeFound, _ = k8s.GetAdmittedRouteHost(ctx, dynClient, interceptorNamespace(), interceptorRouteName(name, namespace))
		}
	}

pkg/k8s/deployer.go:368

  • When Route removal is forbidden and enforce=false, this warns that the Route is being left in place, but then still clears RouteHostnameAnnotation. That makes list/describe report the function as not externally exposed even though the Route likely still exists (and you explicitly said you’re leaving it). In the forbidden/non-enforced path, skip clearing the annotation (or only clear it when the Route was actually removed).
	if _, err := RemoveManagedRoute(ctx, dynClient, namespace, name, KubernetesDeployerName); err != nil {
		if !enforce && errors.IsForbidden(err) {
			fmt.Fprintf(os.Stderr, "⚠️  cannot remove Route %q (forbidden) - leaving it in place\n", name)
		} else {
			return fmt.Errorf("failed to remove Route: %w", err)
		}
	}

@lkingland

lkingland commented Jul 28, 2026

Copy link
Copy Markdown
Member

"Push lqomtzmulusl #3973"

🤣 🤖

@gauron99 gauron99 changed the title Push lqomtzmulusl feat: functions exposed via ocp-route - keda + raw deployers Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved 🤖 PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. size/XXL 🤖 PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants