Skip to content

knative: drop dead configmap filters#3378

Merged
google-oss-prow[bot] merged 1 commit into
kubeflow:masterfrom
danish9039:knative-yq-followup
Mar 3, 2026
Merged

knative: drop dead configmap filters#3378
google-oss-prow[bot] merged 1 commit into
kubeflow:masterfrom
danish9039:knative-yq-followup

Conversation

@danish9039

@danish9039 danish9039 commented Mar 3, 2026

Copy link
Copy Markdown
Member

✏️ Summary of Changes

This PR is a small follow-up to #3375 (comment) and removes two dead yq eval filters from scripts/synchronize-knative-manifests.sh:

  • config-observability filter on common/knative/knative-eventing/base/upstream/in-memory-channel.yaml
  • config-tracing filter on common/knative/knative-eventing/base/upstream/in-memory-channel.yaml

These lines were originally intended to filter redundant Eventing ConfigMaps from the in-memory channel manifest, but for the current Knative inputs they are no longer affecting the final generated output.

I did not remove the other remaining yq calls:

  • the job-name stabilization still has to stay, because removing it breaks the post-install job bases
  • the comment-stripping calls are not needed for builds, but removing them rewrites several tracked upstream YAML files and would turn this into a larger churn PR

📦 Dependencies

  • Follow-up to #3375
  • Addresses mentor follow-up comment: #issuecomment-3986377704

🐛 Related Issues

  • Knative sync-script cleanup after the explode(.) removal in #3375

✅ Contributor Checklist

  • I have tested these changes locally with kustomize.
  • All commits are signed-off to satisfy the DCO check.
  • I have considered adding my company to the adopters page to support Kubeflow and help the community, since I expect help from the community for my issue.

Local Validation

Validated in a clean worktree and disposable repos:

  • inventory of all remaining yq eval calls in scripts/synchronize-knative-manifests.sh
  • disposable-repo A/B runs against current upstream/master
  • kustomize v5.8.1 build on:
    • common/knative/knative-serving/base
    • common/knative/knative-eventing/base
    • common/knative/knative-serving-post-install-jobs/base
    • common/knative/knative-eventing-post-install-jobs/base

Observed result:

  • removing these two filter lines caused no output drift
  • all 4 Knative builds still passed
  • removing the job-name stabilization lines broke both post-install job base builds
  • removing comment stripping caused large tracked YAML churn, so that was intentionally left out of this minimal follow-up

Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Copilot AI review requested due to automatic review settings March 3, 2026 18:44
@github-actions

github-actions Bot commented Mar 3, 2026

Copy link
Copy Markdown

Welcome to the Kubeflow Manifests Repository

Thanks for opening your first PR. Your contribution means a lot to the Kubeflow community.

Before making more PRs:
Please ensure your PR follows our Contributing Guide.
Please also be aware that many components are synchronizes from upstream via the scripts in /scripts.
So in some cases you have to fix the problem in the upstream repositories first, but you can use a PR against kubeflow/manifests to test the platform integration.

Community Resources:

Thanks again for helping to improve Kubeflow.

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

Removes two no-op yq eval filters from the Knative synchronization script that previously attempted to drop redundant Eventing ConfigMaps from the in-memory channel manifest, but no longer match the current upstream YAML.

Changes:

  • Drop the config-observability filter against in-memory-channel.yaml.
  • Drop the config-tracing filter against in-memory-channel.yaml.

@danish9039

Copy link
Copy Markdown
Member Author

@juliusvonkohout Addressed the comment : #3375 (comment) , I checked the remaining yq eval calls more closely. The two filters for config-observability and config-tracing in in-memory-channel.yaml are redundant now and can be removed, but the job-name stabilization still has to stay because removing it breaks the post-install job bases. I did not remove the comment-stripping calls because they are not needed for builds, but dropping them rewrites several tracked upstream YAML files and would turn this into a much larger churn PR.

@juliusvonkohout

Copy link
Copy Markdown
Member

Thank you
/lgtm
/approve

@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juliusvonkohout

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

@google-oss-prow google-oss-prow Bot merged commit 0240749 into kubeflow:master Mar 3, 2026
11 of 13 checks passed
Raakshass added a commit to Raakshass/manifests that referenced this pull request Mar 27, 2026
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants