Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.13 backports 2023-05-28 #25731

Merged
merged 12 commits into from
Jun 2, 2023

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented May 28, 2023

Once this PR is merged, you can update the PR labels via:

$ for pr in 25422 23939 25549 25570 25670 25573 25665 25702 25674; do contrib/backporting/set-labels.py $pr done 1.13; done

@sayboras sayboras requested review from a team as code owners May 28, 2023 01:28
@sayboras sayboras added kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels May 28, 2023
@sayboras
Copy link
Member Author

/test-backport-1.13

jrajahalme and others added 12 commits May 29, 2023 23:09
[ upstream commit bfa4656 ]

Remove logic from Documentation/Makefile that skips building
'update-helm-values' on non-x86 platforms. This limitation is no longer
needed as we use the helm toolbox image, which is available for multiple
architectures.

Fixes: cilium#20236
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit e2ced1c ]

There was only CLI argument. This adds a fragment of Helm values file
to enable BGP Control Plane

Signed-off-by: Matyáš Kroupa <kroupa.matyas@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 0b366e7 ]

It was not clear that only one of BGP and BGP Control plane can be used
at the same time. This adds a note to the documentation about the
implementations.

Fixes: cilium#23817

Signed-off-by: Matyáš Kroupa <kroupa.matyas@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 00ae20d ]

The Virtual Router Attributes link in BGP Control Plane used incorrect
hyperlink.

Signed-off-by: Matyáš Kroupa <kroupa.matyas@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 707a61a ]

This commit is to make sure that only gateways matching cilium
pre-defined controller are re-queued for reconciliation.

Similar logic is applied for HTTPRoute as well i.e. ignore HTTPRoute if
there is no parent matching cilium controller

Testing was done with the below spec, the expected behavior is no-op
as the controllerName of GatewayClass of Gateway of underlying HTTPRoute
is not same as pre-defined cilium controller name value (e.g.
io.cilium-gateway-controller)

```yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: GatewayClass
metadata:
  name: cilium
spec:
  controllerName: io.cilium/gateway-controller-non-matching

Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit be2306d ]

This is to correct the typo (i.e. l7Proxy instead of l7proxy)
in helm validation if Ingress or Gateway API is enabled. Negative
testing is done as per below

```
$ helm template --namespace kube-system cilium "./install/kubernetes/cilium" --set ingressController.enabled=true --set l7Proxy=false
Error: execution error at (cilium/templates/validate.yaml:52:9): Ingress or Gateway API controller requires .Values.l7Proxy to be set to 'true'

$ helm template --namespace kube-system cilium "./install/kubernetes/cilium" --set gatewayAPI.enabled=true --set l7Proxy=false
Error: execution error at (cilium/templates/validate.yaml:52:9): Ingress or Gateway API controller requires .Values.l7Proxy to be set to 'true'

```

Fixes: ea404cf

Reported-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit d919b30 ]

The "K8sDatapathServicesTest Checks N/S loadbalancing With host policy Tests
NodePort" test has been failing more than 75% of the test runs, thus it will
be quarantined.

Test runs with the failures:

 - https://jenkins.cilium.io/view/all/job/cilium-main-k8s-1.26-kernel-net-next/25
 - https://jenkins.cilium.io/view/all/job/cilium-main-k8s-1.26-kernel-net-next/26
 - https://jenkins.cilium.io/view/all/job/cilium-main-k8s-1.26-kernel-net-next/27
 - https://jenkins.cilium.io/view/all/job/cilium-main-k8s-1.26-kernel-net-next/28
 - https://jenkins.cilium.io/view/all/job/cilium-main-k8s-1.26-kernel-net-next/33

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit 8507ecd ]

There was a race condition between http/tls routes and the underlying
Gateway resource. Basically, if the HTTP or TLS Route resource is created
before Gateway resource, its status will be updated with the value
`gateway not found`. However, the reconciliation for HTTP or TLS routes
is not triggered even after mentioned Gateway is created. The issue is due
to an unnecessary predicate on GenerationChangedPredicate, which only
checks for changes in spec.

This commit is to remove GenerationChangedPredicate predicate in HTTP and
TLS routes.

Status for gateway not found

```json
{
  "lastTransitionTime": "2023-05-22T05:24:01Z",
  "message": "Gateway.gateway.networking.k8s.io \"gateway-tlsroute\" not found",
  "observedGeneration": 1,
  "reason": "InvalidTLSRoute",
  "status": "False",
  "type": "Accepted"
}
```

Testing was done before and after the changes with below step:

- Create one TLSRoute with a non-existent Gateway. This TLSRoute status should say `gateway not found`
- Create mentioned Gateway Resource with a valid GatewayClass
- Verify the TLSRoute status, which should be updated to Accepted.

Fixes: cilium#25487
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit a0bfd5d ]

The entity `ingress` is missing from the list of pre-defined entities
which are available when defining policies which `fromEntities` and
`toEntities`.

This commits fixes this.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit eb5bf06 ]

The test case was introduced to cover issue cilium#21954, but it turned out
the test is buggy and caused a number of CI flakes (cilium#25119).
Consequently, PR cilium#25236 put the test case under quarantine.

This commit removes that problematic test, as the target scenario has
been covered by connectivity test in cilium-cli
(cilium/cilium-cli#1547).

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit e8fcd6b ]

Envoy by default gets the source address from the `x-forwarded-for`
header, if present. Always add an explicit `use_remote_address: true` for
Envoy HTTP Connection Manager configuration to disable the default
behavior.

Also set the `skip_xff_append: true` option to retain the old behavior of
not adding `x-forwarded-for` headers on cilium envoy proxy.

Setting these options is not really needed for admin and metrics
listeners, or most of the tests, but we add them there too in case anyone
uses them as a source of inspiration for a real proxy configuration.

This fixes incorrect hubble flow data when HTTP requests contain an
`x-forwarded-for` header. This change has no effect on Cilium policy
enforcement where the source security identity is always resolved before
HTTP headers are parsed.

Fixes: cilium#25630
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
[ upstream commit abb355e ]

Do not skip adding `x-forwarded-for` in Cilium Ingress.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras force-pushed the pr/v1.13-backport-2023-05-28 branch from d7b030f to 8ee2cd2 Compare May 29, 2023 13:09
@sayboras
Copy link
Member Author

/test-backport-1.13

Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for backporting tam!

@sayboras
Copy link
Member Author

/test-runtime

@sayboras sayboras removed the request for review from NikAleksandrov May 30, 2023 15:43
@sayboras sayboras requested a review from a team June 1, 2023 09:13
@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 1, 2023
@sayboras
Copy link
Member Author

sayboras commented Jun 1, 2023

most of the reviews are in, all tests passed. Marking this ready to merge.

@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2023
@julianwiedmann julianwiedmann merged commit 769f1bb into cilium:v1.13 Jun 2, 2023
62 checks passed
@sayboras sayboras deleted the pr/v1.13-backport-2023-05-28 branch June 2, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants