OCPBUGS-104452: Fix TLS ciphers for MinVersion=1.3 - #135
Conversation
Allow FRRConfiguration users to set an advertised BGP next-hop through the typed toAdvertise API. The new field carries separate IPv4 and IPv6 addresses and converts them into route-map next-hop statements during FRR rendering. Reject invalid next-hop addresses and cases where the requested next-hop family is not enabled for the neighbor. When multiple FRRConfigurations merge into the same neighbor, keep matching values and fail on conflicting next-hop settings. Regenerate deepcopy methods, CRDs, chart CRDs, all-in-one manifests, and API docs, and add conversion, merge, and render coverage. Signed-off-by: Tim Rozet <trozet@nvidia.com>
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Add EVPN-related types to the v1beta1 API: EVPNConfig, VNI, L2VNI, L3VNI, RouteTarget, RouteDistinguisher, AddressFamily, and AdvertisePrefixType. A Router can now have an optional EVPN config with L2/L3 VNIs, route targets, route distinguishers, and VNI advertisement control. Neighbors gain an AddressFamilies field to activate them for unicast, EVPN, or both. Includes CRD validation tests and regenerated CRD manifests, deepcopy, and API docs. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Add internal representation types (EVPNConfig, L2VNI, L3VNI) to the frr package config, mirroring the API types with string-based fields suitable for template rendering. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Implement merging of EVPN configurations from multiple FRRConfiguration resources. Validates compatibility of advertiseVNIs and advertiseSVI settings, merges L2/L3 VNI lists checking for conflicts on VNI numbers, and merges neighbor address families. Includes unit tests covering compatible merges, conflicts, and nil handling. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Translate EVPN API types to internal FRR config types, including L2VNIs, L3VNIs, route targets, route distinguishers, and advertise prefix types. Adds validation that EVPN config is only present on routers that have neighbors with the EVPN address family. Includes unit tests for the translation and validation logic. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Add evpn.tmpl template and integrate it into the main FRR config template. Renders L2VPN EVPN address family activation per neighbor, advertise-all-vni, advertise-svi-ip, per-VNI route distinguisher and route target configuration, VNI-VLAN mappings, and L3 VNI settings with type-5 route advertisement. Includes golden file tests for full EVPN, L2-only, L3-only, and EVPN-only neighbor scenarios. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Add envtest-based controller tests verifying end-to-end EVPN configuration flow from FRRConfiguration CRD to rendered FRR config, including L2 VNIs, L3 VNIs, and multi-resource merge scenarios. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Add end-to-end tests for EVPN using external FRR containers configured with VXLAN interfaces and bridge VLANs via hack/evpn-setup.sh. Tests validate L2 VNI and L3 VNI BGP route advertisement by inspecting FRR's EVPN route tables, consistent with the existing control-plane test approach. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Document EVPN support including L2 VNI, L3 VNI, and combined L2+L3 configuration examples. Covers prerequisites, VNI configuration fields, route targets, route distinguishers, and type-5 prefix advertisement. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Implementation of the EVPN design proposal
Four independent bugs in `charts/frr-k8s/values.schema.json` that
together leave strictly-shaped chart inputs unvalidated, reject
documented inputs, and bury a large `frrk8s` block in a schema-ignored
position. Single-file diff.
1. JSON property key typo: `"resources:"` (with trailing colon).
Pre-fix the property name under `frrk8s.frr.properties` had a stray
colon, so the `resources` map under `frrk8s.frr` was effectively
unschema'd. Rename to `"resources"`, matching the sibling shapes
`frrk8s.reloader.resources`, `frrk8s.frrMetrics.resources`,
`frrk8s.frrStatus.resources` in the same file.
2. `prometheus.serviceMonitor.interval` schema mismatch.
Pre-fix declared `anyOf: integer | null`, but the Prometheus
convention for scrape intervals is a duration string (`"30s"`,
`"1m"`, etc.) and `charts/frr-k8s/README.md:86` already documents
the field as `string`. Operators who set
`prometheus.serviceMonitor.interval: 30s` were rejected by
`helm template` with a schema error. Widen to
`anyOf: string | null`.
3. `crds` block misplaced under `prometheus`.
Pre-fix the `crds` schema lived at `properties.prometheus.crds`.
JSON Schema silently ignores unknown sibling keys, so the entire
top-level `crds` map in `values.yaml` was unvalidated —
`crds.enabled: "not_a_bool"` was accepted without complaint. Move
it to `properties.crds`; the schema now catches type errors on
`crds.enabled` and constrains `crds.validationFailurePolicy` to
the documented enum `Ignore | Fail`.
4. `frrk8s` block misplaced under `prometheus`.
Pre-fix an entire `frrk8s` definition (`tolerateMaster`,
`updateStrategy`, `runtimeClassName`, `secretName`, `frr.{image,
metricsPort, secureMetricsPort, resources}`, `command`, `reloader`,
`frrMetrics`) lived at `properties.prometheus.frrk8s` — a sibling
of `properties` inside the `prometheus` block. Same JSON-Schema
silent-ignore as openshift#3: the chart's actual `frrk8s.*` values went
unvalidated. Merge the misplaced block into the existing top-level
`properties.frrk8s` (which previously held only `strategy`,
`command`, `webhookMode`). The `frr.required: [enabled]` rule is
dropped during the move because the chart's own `values.yaml` does
not set `frrk8s.frr.enabled` — enforcing it would reject the chart's
default render with a confusing "missing property" error, which the
pre-fix schema avoided by being in a position the validator
ignored.
Verified locally (helm v3.18 and v3.21):
Pre-fix:
- `helm template ... --set prometheus.serviceMonitor.interval=30s` →
rejected ("got string, want integer").
- `helm template ... --set crds.enabled=not_a_bool` → silently accepted.
- `helm template ... --set frrk8s.tolerateMaster=not_a_bool` → silently
accepted.
Post-fix:
- `interval=30s` → accepted.
- `crds.enabled=not_a_bool` → rejected:
`at '/crds/enabled': got string, want boolean`.
- `frrk8s.tolerateMaster=not_a_bool` → rejected:
`at '/frrk8s/tolerateMaster': got string, want boolean`.
- Default `helm template charts/frr-k8s --set crds.enabled=true` renders
cleanly (42 documents, 0 errors).
- `helm lint charts/frr-k8s --set crds.enabled=true` clean.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The schema declared fullNameOverride but values.yaml, templates, and README all use fullnameOverride (lowercase n), so the property went unvalidated. Rename to match. Verified locally (helm v3.12.3): Pre-fix: - `helm template ... --set-json fullnameOverride=123` -> not caught by the schema; fails later during template rendering with a cryptic type error (_helpers.tpl:16:37 ... wrong type for value; expected string; got float64). - `helm template ... --set-json fullNameOverride=123` -> rejected by the schema, i.e. it was validating a key the chart never reads. Post-fix: - `helm template ... --set-json fullnameOverride=123` -> rejected by the schema: `fullnameOverride: Invalid type. Expected: string, given: integer`. - `helm template ... --set fullnameOverride=my-frrk8s` -> renders cleanly (48 manifests). - `helm template charts/frr-k8s --set crds.enabled=true` -> clean default render. - `helm lint charts/frr-k8s` -> clean. Signed-off-by: Riccardo Ravaioli <rravaioli@nvidia.com>
Allow setting resource requests/limits on the frr-k8s init containers via Helm values. Signed-off-by: Simon Li <li@siutsin.com>
Only add prometheus.io/scrape to the metrics Service when prometheus.scrapeAnnotations is enabled. This avoids duplicate scrapes when users enable the ServiceMonitor and also have annotation-based discovery configured. Signed-off-by: Soli0222 <github@str08.net>
Build the metrics Service annotations map before rendering it so the chart avoids duplicate keys and keeps YAML indentation stable. Signed-off-by: Soli0222 <github@str08.net>
Add support for the BGP allowas-in neighbor directive, controlled via a new AllowAsIn field on the Neighbor configuration. This allows accepting routes that contain the local AS number in the AS path, which is useful in hub-and-spoke or route-leaking topologies. Possible values: "" or "none" (reject, default), "origin" (accept only at origin), "any" (accept regardless of occurrences). The directive is applied to all enabled address families (ipv4/ipv6 unicast and l2vpn evpn). When multiple configurations target the same neighbor, "none" explicitly prevents any other configuration from enabling allowas-in. Any other combination resolves to the least restrictive. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Restructure EVPN tests from a flat DescribeTableSubtree into a
Describe("EVPN") with two siblings: the existing iBGP VNI subtree
and a new AllowAsIn DescribeTable. The AllowAsIn tests verify that
EVPN type-5 routes with local AS in path are accepted with
AllowAsIn origin and rejected without it.
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
…container helm/controller: use exec form for FRR tini command to prevent shell injection Replace the /bin/sh -c wrapper with a direct exec-form command list so tiniPath and dockerStartPath values are never interpreted by a shell. Make FRR container security context configurable. Signed-off-by: Peter Benis <benispeti@gmail.com>
We were too harsh by denying configurations for suites when tls min version is 1.3, as go itself does not deny them but rather ignores them. Here we align with this approach. Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@oribon: This pull request references Jira Issue OCPBUGS-104452, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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-ci
Bot
added
the
approved
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
/testwith openshift/frr/main/frrk8s-e2e-metal openshift/cluster-network-operator#3108 |
|
/override ci/prow/okd-scos-images #136 confirms okd-scos-images is unrelated, will be tracked elsewhere |
|
@oribon: Overrode contexts on behalf of oribon: ci/prow/okd-scos-images 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. |
|
/testwith openshift/frr/main/frrk8s-e2e-metal openshift/cluster-network-operator#3108 |
1 similar comment
|
/testwith openshift/frr/main/frrk8s-e2e-metal openshift/cluster-network-operator#3108 |
|
/testwith openshift/frr/main/frrk8s-e2e-metal openshift/cluster-network-operator#3108 |
1 similar comment
|
/testwith openshift/frr/main/frrk8s-e2e-metal openshift/cluster-network-operator#3108 |
|
/override ci/prow/okd-scos-images |
|
/verified by ci |
|
@oribon: Overrode contexts on behalf of oribon: ci/prow/okd-scos-images 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. |
openshift-ci-robot
added
the
verified
|
@oribon: 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. |
|
/lgtm |
|
Scheduling required tests: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: oribon, SchSeba 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 |
|
/override ci/prow/frrk8s-e2e the first one will not pass until both cno and the skip prs are in |
|
@oribon: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
|
/override ci/prow/frrk8s-e2e-metal |
|
@oribon: Overrode contexts on behalf of oribon: ci/prow/frrk8s-e2e-metal, ci/prow/metallb-e2e-metal 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. |
|
@oribon: Jira Issue OCPBUGS-104452: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged: All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-104452 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload. 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. |
|
@oribon: 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. |
11 participants
No description provided.