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

Bump up opentelemetry to avoid scanner CVE warnings #5703

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

liu4480
Copy link
Contributor

@liu4480 liu4480 commented Nov 14, 2023

For CVE-2023-47108 and CVE-2023-45142.

These CVEs don't affect Antrea, including prior versions of Antrea.
This patch is only meant to avoid warnings from CVE scanners.

See also #5602

Signed-off-by: Bin Liu biliu@vmware.com

@liu4480
Copy link
Contributor Author

liu4480 commented Nov 14, 2023

/test-conformance
/test-e2e
/test-networkpolicy

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

Is it typical to bump up indirect dependencies directly? Could it be overwriten when the direct dependency requiring it upgrades in the future?
cc @antoninbas

@antoninbas
Copy link
Contributor

@tnqn This is fine. If a direct dependency upgrades and starts requiring a minimum version that > to the one we have in the go.mod, the indirect entry in our go.mod will be automatically updated to that new minimum version.

Typically the main concern is that you should not upgrade to a new major version, as that new major version may not be compatible with your direct dependency. The same concern can apply for pre-v1 minor version updates, if backward compatibility is broken. However, even though we have some major version updates here, it seems that everything still builds fine. In theory, it's possible to have some broken functionality, in case of a behavior change of the library. But we don't really leverage OTEL in the Antrea apiservers, and we don't provide a way to enable tracing:

In K8s v1.26, the APIServerTracing FeatureGate is Alpha, and disabled by default. This means that when we import version v0.26.4 of the apiserver module, the FeatureGate is automatically disabled, and it is not possible for users to enable it:
https://github.com/kubernetes/apiserver/blob/d3e415393e7e3642d9e6bb2a445385fd5e0e10bd/pkg/features/kube_features.go#L79

When we upgrade our K8s dependencies in the future, maybe it will make more sense to revert these changes and let K8s choose OTEL module versions.

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

This is fine by me. However, the commit message and PR description should make it clear that this is only to make CVE scanners happy, as these CVEs don't affect Antrea, including prior versions of Antrea. The current title, with the use of "fix", kind of implies that Antrea was affected by these CVEs. See #5602 (may be a good idea to reference the issue in the commit message as well).

We probably do not need to backport this to supported Antrea versions either. I know that K8s is not back-porting their PR (kubernetes/kubernetes#121338), as per their policy: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-release/cherry-picks.md

…and CVE-2023-45142

Refer to issue antrea-io#5602, these CVEs don't affect Antrea, including prior versions of Antrea.
This patch only removes warning from CVE scanners.

Signed-off-by: Bin Liu <biliu@vmware.com>
@liu4480 liu4480 changed the title bump up opentelemetry to fix CVE-2023-47108 and CVE-2023-45142 bump up opentelemetry to make CVE scanners not wanring CVE-2023-47108 and CVE-2023-45142 Nov 15, 2023
@antoninbas antoninbas changed the title bump up opentelemetry to make CVE scanners not wanring CVE-2023-47108 and CVE-2023-45142 bump up opentelemetry to avoid scanners warnings about CVE-2023-47108 and CVE-2023-45142 Nov 15, 2023
@antoninbas antoninbas changed the title bump up opentelemetry to avoid scanners warnings about CVE-2023-47108 and CVE-2023-45142 Bump up opentelemetry to avoid scanner CVE warnings Nov 15, 2023
@antoninbas
Copy link
Contributor

/test-all

@tnqn
Copy link
Member

tnqn commented Nov 16, 2023

/skip-e2e

@tnqn
Copy link
Member

tnqn commented Nov 16, 2023

@tnqn This is fine. If a direct dependency upgrades and starts requiring a minimum version that > to the one we have in the go.mod, the indirect entry in our go.mod will be automatically updated to that new minimum version.

Typically the main concern is that you should not upgrade to a new major version, as that new major version may not be compatible with your direct dependency. The same concern can apply for pre-v1 minor version updates, if backward compatibility is broken. However, even though we have some major version updates here, it seems that everything still builds fine. In theory, it's possible to have some broken functionality, in case of a behavior change of the library. But we don't really leverage OTEL in the Antrea apiservers, and we don't provide a way to enable tracing:

In K8s v1.26, the APIServerTracing FeatureGate is Alpha, and disabled by default. This means that when we import version v0.26.4 of the apiserver module, the FeatureGate is automatically disabled, and it is not possible for users to enable it:
https://github.com/kubernetes/apiserver/blob/d3e415393e7e3642d9e6bb2a445385fd5e0e10bd/pkg/features/kube_features.go#L79

When we upgrade our K8s dependencies in the future, maybe it will make more sense to revert these changes and let K8s choose OTEL module versions.

Got it, thanks for explanation.

@tnqn tnqn merged commit bdf2d6b into antrea-io:main Nov 16, 2023
47 of 56 checks passed
antoninbas pushed a commit to antoninbas/antrea that referenced this pull request Mar 26, 2024
We re-apply antrea-io#5703.
When updating K8s dependencies in antrea-io#5843, the change was reverted and we
went back to using an older version of opentelemetry which is affected
by a CVE. The CVE doesn't affect Antrea, the patch is meant to avoid
warnings from CVE scanners.

Co-authored-by: Antonin Bas <antonin.bas@broadcom.com>

Signed-off-by: Bin Liu <biliu@vmware.com>
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
antoninbas added a commit that referenced this pull request Mar 27, 2024
We re-apply #5703.
When updating K8s dependencies in #5843, the change was reverted and we
went back to using an older version of opentelemetry which is affected
by a CVE. The CVE doesn't affect Antrea, the patch is meant to avoid
warnings from CVE scanners.

Signed-off-by: Bin Liu <biliu@vmware.com>
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Co-authored-by: Bin Liu <biliu@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants