[backport/1.22] ocsp: rotate certs of test data to fix flaky tests (#23635) by dcillera · Pull Request #23842 · envoyproxy/envoy

dcillera · 2022-11-04T15:29:12Z

ocsp: rotate certs of test data

Signed-off-by: wbpcode wangbaiping@corp.netease.com

fix format

Signed-off-by: wbpcode wangbaiping@corp.netease.com

KBaichoo · 2022-11-04T17:29:25Z

/assign @phlax

phlax · 2022-11-04T17:39:06Z

i think most likely this will also need the infra fix - if you check this PR - #23817 - you can see the other commit that is likely required

phlax · 2022-11-07T09:34:51Z

thanks for updating @dcillera could you fix DCO please

im surprised that DCO is failing as the commits are signed by the original contributors - but it seems like it wants your signature also

dcillera · 2022-11-07T09:52:17Z

Fixed DCO problem by performing suggested procedure.

phlax

lgtm, thanks @dcillera

phlax · 2022-11-07T10:27:19Z

arggh - seems like time has warped on this branch and one of the dependencies was released on a different date

https://dev.azure.com/cncf/envoy/_build/results?buildId=119875&view=logs&j=1c04237c-5f52-5959-5f83-5522b3556331&t=db3f3889-c922-566b-fa2e-4bc4948bc467&l=969

im just looking at this branch so im happy to fix - but if you add the fix here first i will land

dcillera · 2022-11-07T11:43:56Z

I think the following commit is also missing. Do you think we should add it to this PR?

commit b1f5e92
Author: Xie Zhihao zhihao.xie@intel.com
Date: Fri Sep 23 18:13:47 2022 +0800

deps: fix mismatch release date in OpenCensus Proto (#23220)

Signed-off-by: Xie Zhihao <zhihao.xie@intel.com>

phlax · 2022-11-07T11:49:21Z

yep - that looks like the fix i think

repokitteh-read-only · 2022-11-07T11:51:45Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @phlax

🐱

Caused by: #23842 was synchronize by dcillera.

see: more, trace.

phlax · 2022-11-07T12:00:10Z

i just hit a similar (codeql) problem trying to update the 1.21 branch #23852 (comment)

phlax · 2022-11-07T12:20:55Z

testing a fix for codeql here - #23859

phlax · 2022-11-07T12:40:11Z

@dcillera the codeql problem is caused by the ubuntu image not being pinned - ie its set to ubuntu-latest

im going to fix that on main so its explicit which version its using which should prevent this happening in the future

for this PR do you want to add the following diff/commit:

diff --git a/.github/workflows/codeql-daily.yml b/.github/workflows/codeql-daily.yml
index 612fa1722e..0c37880c7d 100644
--- a/.github/workflows/codeql-daily.yml
+++ b/.github/workflows/codeql-daily.yml
@@ -11,7 +11,7 @@ jobs:
       fail-fast: false
 
     # CodeQL runs on ubuntu-latest and windows-latest
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-18.04
 
     steps:
     - name: Checkout repository
diff --git a/.github/workflows/codeql-push.yml b/.github/workflows/codeql-push.yml
index bb31007938..b6a42b8f7b 100644
--- a/.github/workflows/codeql-push.yml
+++ b/.github/workflows/codeql-push.yml
@@ -17,7 +17,7 @@ jobs:
       fail-fast: false
 
     # CodeQL runs on ubuntu-latest and windows-latest
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-18.04
     if: github.repository == 'envoyproxy/envoy'
 
     steps:

ill test it now - but im fairly confident this is the required fix

phlax · 2022-11-07T13:12:52Z

the release bug in CI looks like a genuine fail

`[2022-11-07 12:57:38.820][17][error][envoy_bug] [source/extensions/transport_sockets/tls/context_impl.cc:443] envoy bug failure: value_stat_name != fallback. Details: Unexpected ssl.ciphers value: unexpected

https://dev.azure.com/cncf/envoy/_build/results?buildId=119887&view=logs&j=8c169225-0ae8-53bd-947f-07cb81846cb5&t=6b0ace90-28b2-51d9-2951-b1fd35e67dec&l=4175

phlax · 2022-11-07T13:54:00Z

im wondering if this commit 6d39a4e is related to the tls failures - merging requires some conflict resolution and not 100% certain if its the issue/fix but looks like it to me

phlax · 2022-11-08T08:43:34Z

hmm seems like my suggestion was either incorrect or incomplete - did you have any joy debugging locally ?

dcillera · 2022-11-08T08:50:05Z

However, I observe that even in 1.23 and 1.24 we have: "runs-on: ubuntu-latest" (and "ubuntu-20.04" in main).
I also tried 6d39a4e
but got a lot of errors.

phlax · 2022-11-08T08:52:58Z

However, I observe that even in 1.23 and 1.24 we have: "runs-on: ubuntu-latest" (and "ubuntu-20.04" in main).

im pretty sure that suggestion was correct -that was just a fix for codeql - the ubuntu version is going to be different on different versions - in 1.21/2 its a case of just setting it to what was latest when the branches were active

the TLS bug in the release CI is more concerning - there are a few possible commits in the 1.23 that could be the fix - im not c++ so figuring out which one is beyond my immediate skills - the suggestion i made on this seems to be wrong

phlax · 2022-11-08T09:04:59Z

re codeql - i have just backported the fix from main to the 1.23 and 1.24 branches - #23878 and #23879

phlax · 2022-11-09T10:46:24Z

@wbpcode @daixiang0 could you help out here

we are trying to update the 1.22 branch with recent fixes from main

if we add just 6d39a4e

we are getting an error:

test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc:560:59: error: member reference base type 'size_t' (aka 'unsigned long') is not a structure o
r union                                                                                                                                                                           
  EXPECT_EQ(19956, validator().daysUntilFirstCertExpires().value());

i then tried picking #20581 and #21428

but that then throws other errors - as i think the code had changed on main differently to this branch - altho looks like easy fixes, beyond my immediate skills (also possible issuse is incorrect merging - not sure)

any help would be much appreciated!

its important to get this branch functioning again

wbpcode · 2022-11-09T10:56:18Z

cc @phlax could just run the refresh script in the 1.22 rather than cherry-pick?

phlax · 2022-11-09T10:59:26Z

cc @phlax could just run the refresh script in the 1.22 rather than cherry-pick?

i could - thanks!

i guess i can find it - but ftr - how do you do that ?

phlax · 2022-11-09T15:40:13Z

ive added a commit here ~~ad0705a~~ eaa67747d928d1934d993665f90dcad6a6491a61 with a possible fix for certs as i wanted to test other backports on this branch - lets see if it works

phlax · 2022-11-09T17:44:44Z

arggh - it didnt but from scan it looks like another openssl version issue - i think i may need to rerun using the envoy build container (and not my dev container)

phlax · 2022-11-10T12:13:47Z

@wbpcode if i just update the certs with the script i hit this:

test/extensions/transport_sockets/tls/context_impl_test.cc:156: Failure
Expected equality of these values:
  context->daysUntilFirstCertExpires()
    Which is: 0
  days_until_expiry
    Which is: -19

which is i think the same problem i had when picking the change from other branch

@daixiang0 any ideas on this - i know you fixed some related issues on main

wbpcode · 2022-11-10T14:08:59Z

cc @phlax Seems like the certs for tls is also outdated, not just oscp.

phlax · 2022-11-10T14:10:20Z

i thought that was what i was trying to fix - this branch already has the oscp update i think - is there a different command we need to run or commit to pick ?

wbpcode · 2022-11-10T14:12:09Z

See #22792 and #22811

wbpcode · 2022-11-10T14:12:51Z

You can update them by the following commands:

$ cd test/extensions/transport_sockets/tls/test_data
# ./certs.sh

please take care the format of the generated .h header file and some unnecessary temporary header files.

phlax · 2022-11-10T14:13:52Z

that is what i did - which is not working

im realising tho that the ocsp cert fix is the same - im wondering if removing that and running the script will fix - ill try on my other branch ...

Previously `/var/run/docker.sock` is readable/writable inside docker run because group ID of `envoygroup` coincidentally matches host docker group, while it is no longer true during rolling out new image. Fixing that by forcing `envoygroup` has host docker group ID. Risk Level: Low Testing: CI Docs Changes: Release Notes: Platform Specific Features: Signed-off-by: Lizan Zhou <lizan@tetrate.io> Signed-off-by: Dario Cillerai <dcillera@redhat.com> Signed-off-by: Ryan Northey <ryan@synca.io>

Signed-off-by: Xie Zhihao <zhihao.xie@intel.com> Signed-off-by: Ryan Northey <ryan@synca.io>

Signed-off-by: Ryan Northey <ryan@synca.io>

phlax · 2022-11-10T14:16:00Z

argghhh - the same result

wbpcode · 2022-11-10T14:17:52Z

I am running the commands and tests in my local env. It need take some times.

wbpcode · 2022-11-10T14:44:25Z

cc @phlax It should be fixed by above two refresh commends (one for tls one for ocsp). And a minor test code update is necessary.

I create a new pr to run the complete CI. See #23931

phlax · 2022-11-10T14:46:27Z

thanks for tracking that down @wbpcode

@dcillera - hopefully the commit in #23842 should fix the certs issue/s

phlax

lgtm, thanks @dcillera for persevering on this

Signed-off-by: wbpcode <wangbaiping@corp.netease.com> Signed-off-by: Ryan Northey <ryan@synca.io>

phlax

/lgtm deps

thanks again @dcillera

repokitteh-read-only · 2022-11-15T11:06:14Z

🙀 Error while processing event:

evaluation error
error: signaled

🐱

Caused by: a #23842 (review) was submitted by @phlax.

see: more, trace.

dcillera requested review from ggreenway and lizan as code owners November 4, 2022 15:29

repokitteh-read-only Bot assigned phlax Nov 4, 2022

phlax added this to the 1.22.6 milestone Nov 4, 2022

dcillera force-pushed the release/v1.22 branch from 0054069 to 36cd4f0 Compare November 7, 2022 09:49

phlax previously approved these changes Nov 7, 2022

View reviewed changes

dcillera dismissed phlax’s stale review via 81adb9d November 7, 2022 11:51

repokitteh-read-only Bot added the deps Approval required for changes to Envoy's external dependencies label Nov 7, 2022

phlax mentioned this pull request Nov 7, 2022

repo: Dev 1.22.6 #23856

Merged

dcillera force-pushed the release/v1.22 branch from bca9a69 to 81adb9d Compare November 7, 2022 18:18

lizan and others added 3 commits November 10, 2022 14:14

deps: fix mismatch release date in OpenCensus Proto (envoyproxy#23220)

aefbba1

Signed-off-by: Xie Zhihao <zhihao.xie@intel.com> Signed-off-by: Ryan Northey <ryan@synca.io>

codeql-fix

15b83c5

Signed-off-by: Ryan Northey <ryan@synca.io>

This was referenced Nov 10, 2022

refresh certs for tls and ocsp tests #23931

Closed

[backport/1.22] Release 1.22 build fix #23951

Merged

[backport/1.22] wasm: update Wasmtime to v0.39.1 #23975

Merged

dcillera requested a review from mattklein123 as a code owner November 14, 2022 17:00

phlax previously approved these changes Nov 14, 2022

View reviewed changes

repokitteh-read-only Bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 14, 2022

refresh certs for tls and ocsp tests

c75043b

Signed-off-by: wbpcode <wangbaiping@corp.netease.com> Signed-off-by: Ryan Northey <ryan@synca.io>

dcillera dismissed phlax’s stale review via c75043b November 15, 2022 08:37

dcillera force-pushed the release/v1.22 branch from a4a8f42 to c75043b Compare November 15, 2022 08:37

repokitteh-read-only Bot added the deps Approval required for changes to Envoy's external dependencies label Nov 15, 2022

phlax approved these changes Nov 15, 2022

View reviewed changes

repokitteh-read-only Bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 15, 2022

phlax enabled auto-merge (rebase) November 15, 2022 11:10

phlax merged commit 3c8c534 into envoyproxy:release/v1.22 Nov 15, 2022

Conversation

dcillera commented Nov 4, 2022

Uh oh!

KBaichoo commented Nov 4, 2022

Uh oh!

phlax commented Nov 4, 2022

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

dcillera commented Nov 7, 2022

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

dcillera commented Nov 7, 2022

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

repokitteh-read-only Bot commented Nov 7, 2022

Uh oh!

phlax commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

phlax commented Nov 7, 2022

Uh oh!

phlax commented Nov 8, 2022

Uh oh!

dcillera commented Nov 8, 2022

Uh oh!

phlax commented Nov 8, 2022

Uh oh!

phlax commented Nov 8, 2022

Uh oh!

phlax commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode commented Nov 9, 2022

Uh oh!

phlax commented Nov 9, 2022

Uh oh!

phlax commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phlax commented Nov 9, 2022

Uh oh!

phlax commented Nov 10, 2022

Uh oh!

wbpcode commented Nov 10, 2022

Uh oh!

phlax commented Nov 10, 2022

Uh oh!

wbpcode commented Nov 10, 2022

Uh oh!

wbpcode commented Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phlax commented Nov 10, 2022

Uh oh!

phlax commented Nov 10, 2022

Uh oh!

wbpcode commented Nov 10, 2022

Uh oh!

wbpcode commented Nov 10, 2022

Uh oh!

phlax commented Nov 10, 2022

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

phlax commented Nov 7, 2022 •

edited

Loading

phlax commented Nov 9, 2022 •

edited

Loading

phlax commented Nov 9, 2022 •

edited

Loading

wbpcode commented Nov 10, 2022 •

edited

Loading