OCPCLOUD-3347: tls: use centralized TLS profile (unrevert)#292
Open
damdo wants to merge 7 commits intoopenshift:mainfrom
Open
OCPCLOUD-3347: tls: use centralized TLS profile (unrevert)#292damdo wants to merge 7 commits intoopenshift:mainfrom
damdo wants to merge 7 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@damdo: This pull request references OCPCLOUD-3347 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
Member
Author
|
/hold For HCP to sort out the RBAC permission issue |
openshift-ci
bot
added
the
do-not-merge/hold
Member
Author
|
/cc @neisw @bryan-cox |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo 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 |
openshift-ci
bot
added
the
approved
damdo
force-pushed
the
use-centralized-tls-profile-2
branch
from
2458331 to
357265a
Compare
Contributor
|
/hold until the hypershift presubmit jobs are in place openshift/release#75309 |
Member
Author
|
/retest |
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aks openshift/hypershift#7802 |
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aws openshift/hypershift#7802 |
Contributor
|
Seems it is still failing |
In HyperShift, shouldn't we be querying the HostedCluster object instead of the APIServer one? At least the enhancement seems to suggest that for SLOs. Edit: Saying that, using APIServer object from the hosted cluster in HyperShift works just fine for my use case and the implementation is simpler than trying to find the HostedCluster object as suggested by the the enhancement. |
Contributor
|
PR needs rebase. 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. |
openshift-merge-robot
added
the
needs-rebase
damdo
force-pushed
the
use-centralized-tls-profile-2
branch
from
357265a to
9ee0b44
Compare
WalkthroughThis PR introduces native TLS support to the machine-approver controller by replacing the external kube-rbac-proxy sidecar with built-in metrics server TLS configuration. It adds CLI flags for TLS settings, implements dynamic TLS profile resolution based on cluster APIServer configuration, updates deployment manifests accordingly, and bumps Kubernetes/OpenShift dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
removed
the
needs-rebase
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tls.go`:
- Around line 90-97: The current lookups using
utiltls.FetchAPIServerTLSAdherencePolicy and utiltls.FetchAPIServerTLSProfile
treat a NotFound as fatal and block controller startup; change these calls to
treat api server "cluster" NotFound as a non-fatal fallback by detecting the
NotFound error (apierrors.IsNotFound(err)) and continuing with
default/topology-specific TLS settings instead of returning fmt.Errorf; keep
returning errors for other error types. Update the code around
tlsAdherencePolicy and tlsProfileSpec retrieval to branch on
apierrors.IsNotFound(err) and document that defaults are used when missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e11ae30-6239-4703-941a-f55d90519025
⛔ Files ignored due to path filters (291)
go.sumis excluded by!**/*.sumvendor/cel.dev/expr/.bazelversionis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/CODE_OF_CONDUCT.mdis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/GOVERNANCE.mdis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/MAINTAINERS.mdis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/README.mdis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/WORKSPACEis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/WORKSPACE.bzlmodis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/cel.dev/expr/cloudbuild.yamlis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/cel.dev/expr/regen_go_proto.shis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/regen_go_proto_canonical_protos.shis excluded by!vendor/**,!**/vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/antlrdoc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_config_set.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_deserialization_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_deserializer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_simulator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_state.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/atn_type.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/char_stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/common_token_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/common_token_stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/comparators.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/configuration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/dfa.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/dfa_serializer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/dfa_state.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/diagnostic_error_listener.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/error_listener.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/error_strategy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/file_stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/input_stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/int_stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/interval_set.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/jcollect.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/lexer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/lexer_action.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/lexer_action_executor.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/lexer_atn_simulator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/ll1_analyzer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/nostatistics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/parser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/parser_atn_simulator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/parser_rule_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/prediction_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/prediction_context_cache.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/prediction_mode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/recognizer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/rule_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/semantic_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/statistics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/stats_data.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/token_source.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/token_stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/tokenstream_rewriter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/trace_listener.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/transition.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/tree.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/trees.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/antlr4-go/antlr/v4/utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/backoff.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/exponential.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/retry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/ticker.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/timer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v4/tries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/capture_metrics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/docs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/wrap_generated_gteq_1.8.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/felixge/httpsnoop/wrap_generated_lt_1.8.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-logr/stdr/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-logr/stdr/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-logr/stdr/stdr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/cel.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/decls.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/env.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/folding.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/inlining.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/io.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/library.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/macro.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/optimizer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/program.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/prompt.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/templates/authoring.tmplis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/cel/validator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/checker.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/cost.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/decls/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/decls/decls.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/env.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/format.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/mapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/printer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/scopes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/checker/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/ast/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/ast/ast.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/ast/conversion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/ast/expr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/ast/factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/ast/navigable.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/containers/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/containers/container.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/cost.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/debug/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/debug/debug.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/decls/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/decls/decls.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/env/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/env/env.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/functions/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/functions/functions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/location.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/operators/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/operators/operators.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/overloads/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/overloads/overloads.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/runes/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/runes/buffer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/source.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/stdlib/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/stdlib/standard.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/any_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/bool.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/bytes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/compare.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/double.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/duration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/err.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/format.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/int.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/iterator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/json_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/list.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/map.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/null.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/object.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/optional.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/overflow.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/checked.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/enum.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/equal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/pb.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/pb/type.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/ref/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/ref/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/ref/reference.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/string.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/timestamp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/comparer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/container.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/field_tester.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/indexer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/iterator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/lister.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/mapper.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/math.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/receiver.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/sizer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/traits.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/traits/zeroer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/uint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/unknown.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/common/types/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/bindings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/comprehensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/encoders.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/extension_option_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/formatting.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/formatting_v2.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/guards.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/lists.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/math.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/native.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/protos.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/regex.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/sets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/ext/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/activation.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/attribute_patterns.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/attributes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/decorators.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/dispatcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/evalstate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/functions/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/functions/functions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/interpretable.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/interpreter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/optimizations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/planner.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/prune.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/interpreter/runtimecost.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/BUILD.bazelis excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/CEL.g4is excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/CEL.interpis excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/CEL.tokensis excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/CELLexer.interpis excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/CELLexer.tokensis excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/cel_base_listener.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/cel_base_visitor.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/cel_lexer.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/cel_listener.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/cel_parser.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/cel_visitor.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/doc.gois excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/gen/generate.shis excluded by!**/gen/**,!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/helper.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/input.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/macro.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/parser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/unescape.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cel-go/parser/unparser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule/compile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule/fuzz.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule/parse.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/convert.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/fieldmask.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_httpbodyproto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_json.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_proto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshaler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshaler_registry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/pattern.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/proto2_convert.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/query.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/utilities/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/utilities/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/utilities/pattern.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/utilities/readerfactory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/utilities/string_array_flag.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/utilities/trie.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (9)
go.modmain.gomanifests/01-rbac.yamlmanifests/02-kube-rbac-proxy-config.yamlmanifests/04-deployment-capi.yamlmanifests/04-deployment.yamlmanifests/image-referencespkg/metrics/metrics.gotls.go
💤 Files with no reviewable changes (2)
- manifests/image-references
- manifests/02-kube-rbac-proxy-config.yaml
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aks openshift/hypershift#8019 |
Member
Author
|
/test e2e-aws-capi-techpreview |
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aws openshift/hypershift#8019 |
damdo
force-pushed
the
use-centralized-tls-profile-2
branch
from
0bb07b2 to
3cf84c8
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
go.mod (2)
56-56:⚠️ Potential issue | 🔴 CriticalUpgrade vulnerable gRPC dependency (critical).
Line 56 pins
google.golang.org/grpc v1.72.2, which is vulnerable to GHSA-p77j-4mvh-x3m3 (authorization bypass). Please upgrade tov1.79.3or later.Suggested diff
- google.golang.org/grpc v1.72.2 // indirect + google.golang.org/grpc v1.79.3 // indirect🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 56, Update the pinned gRPC module version in go.mod: replace the current module entry "google.golang.org/grpc v1.72.2" with at least "google.golang.org/grpc v1.79.3" (or newer) to address the GHSA-p77j-4mvh-x3m3 authorization bypass; after updating the module line run your usual Go dependency update (e.g., go get/resolve and go mod tidy) and run tests/build to ensure no downstream breakage.
47-47:⚠️ Potential issue | 🟠 MajorUpgrade vulnerable OpenTelemetry SDK dependency.
Line 47 pins
go.opentelemetry.io/otel/sdk v1.36.0, which is in the vulnerable range for GO-2026-4394 / GHSA-9h8m-3fm2-qjrq. Please bump tov1.40.0or later before release.Suggested diff
- go.opentelemetry.io/otel/sdk v1.36.0 // indirect + go.opentelemetry.io/otel/sdk v1.40.0 // indirect🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 47, The go.mod currently pins the vulnerable dependency go.opentelemetry.io/otel/sdk at v1.36.0; update that version to v1.40.0 or later (e.g., change go.opentelemetry.io/otel/sdk v1.36.0 -> v1.40.0) and then refresh modules (run the appropriate go get/go mod tidy to ensure transitive updates) so the project uses the non-vulnerable OpenTelemetry SDK release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Line 56: Update the pinned gRPC module version in go.mod: replace the current
module entry "google.golang.org/grpc v1.72.2" with at least
"google.golang.org/grpc v1.79.3" (or newer) to address the GHSA-p77j-4mvh-x3m3
authorization bypass; after updating the module line run your usual Go
dependency update (e.g., go get/resolve and go mod tidy) and run tests/build to
ensure no downstream breakage.
- Line 47: The go.mod currently pins the vulnerable dependency
go.opentelemetry.io/otel/sdk at v1.36.0; update that version to v1.40.0 or later
(e.g., change go.opentelemetry.io/otel/sdk v1.36.0 -> v1.40.0) and then refresh
modules (run the appropriate go get/go mod tidy to ensure transitive updates) so
the project uses the non-vulnerable OpenTelemetry SDK release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8e5b3fe-f184-429f-8d51-efc79a4c129f
📒 Files selected for processing (3)
go.modmain.gotls.go
✅ Files skipped from review due to trivial changes (1)
- tls.go
🚧 Files skipped from review as they are similar to previous changes (1)
- main.go
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aws openshift/hypershift#8019 |
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aks openshift/hypershift#8019 |
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aws openshift/hypershift#8019 |
Member
Author
|
/testwith openshift/cluster-machine-approver/main/e2e-hypershift-aks openshift/hypershift#8019 |
Member
Author
|
/payload-job-with-prs e2e-hypershift-aws openshift/hypershift#8019 |
Contributor
|
@damdo: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
Member
Author
|
/payload-job-with-prs pull-ci-openshift-cluster-machine-approver-main-e2e-hypershift-aws openshift/hypershift#8019 |
Contributor
|
@damdo: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
/payload-job-with-prs periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance openshift/hypershift#8019 |
Contributor
|
@sunzhaohua2: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/15948fb0-244f-11f1-83e2-50fda993a409-0 |
|
/payload-job-with-prs periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-external-oidc openshift/hypershift#8019 |
Contributor
|
@sunzhaohua2: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/15022440-267f-11f1-9a65-bf72365d4b33-0 |
Member
Author
|
/override ci/prow/e2e-aws-capi-techpreview This job is failing for unrelated reasons. And because of the recent changes in openshift/cluster-capi-operator#494 |
Contributor
|
@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-aws-capi-techpreview 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/cluster-machine-approver/main/e2e-hypershift-aws openshift/hypershift#8019 |
damdo
force-pushed
the
use-centralized-tls-profile-2
branch
from
5720894 to
9ab6219
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/tls/tls.go (1)
76-88: Consider the behavior when only cipher suites are provided without a minimum version.When
tlsMinVersionis empty buttlsCipherSuitesis non-empty,cliflag.TLSVersion("")returns0, resulting incfg.MinVersion = 0(Go's default, which is TLS 1.0 for servers). While this may be intentional to allow fine-grained control, operators might expect a reasonable default minimum version when specifying cipher suites.If this is the intended behavior, consider documenting it in the flag help text. Otherwise, consider defaulting to TLS 1.2 when cipher suites are provided but no minimum version is specified.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/tls/tls.go` around lines 76 - 88, The code currently sets cfg.MinVersion from minVersion (which can be 0 when tlsMinVersion flag is empty), causing servers to use Go's default TLS 1.0; update the TLSConfig closure so that if minVersion == 0 and cipherSuites is non-empty you set minVersion = tls.VersionTLS12 before assigning cfg.MinVersion and applying cfg.CipherSuites (refer to the TLSConfig func closure, cfg.MinVersion, minVersion and cipherSuites); this ensures a reasonable default (TLS 1.2) when users supply cipher suites but omit tlsMinVersion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/tls/tls.go`:
- Around line 76-88: The code currently sets cfg.MinVersion from minVersion
(which can be 0 when tlsMinVersion flag is empty), causing servers to use Go's
default TLS 1.0; update the TLSConfig closure so that if minVersion == 0 and
cipherSuites is non-empty you set minVersion = tls.VersionTLS12 before assigning
cfg.MinVersion and applying cfg.CipherSuites (refer to the TLSConfig func
closure, cfg.MinVersion, minVersion and cipherSuites); this ensures a reasonable
default (TLS 1.2) when users supply cipher suites but omit tlsMinVersion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed3bc660-3b1d-4672-ad43-884a0e539a08
📒 Files selected for processing (4)
main.gopkg/tls/suite_test.gopkg/tls/tls.gopkg/tls/tls_test.go
|
/test e2e-aws-capi-techpreview |
Contributor
|
@damdo: The following tests failed, say
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. |
Member
Author
|
@sunzhaohua2 ci/prow/e2e-aws-capi-techpreview failures are unrelated and permanent due to how CCAPIO does handle adoption of CRDs. @mdbooth is working on a fix for that. For now we can override it /override ci/prow/e2e-aws-capi-techpreview |
Contributor
|
@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-aws-capi-techpreview 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. |
Contributor
|
This should be able to merge before the hypershift change (openshift/hypershift#8019) so that we can properly premerge test for both PRs. The issue is that the PR is still trying to read the APIServer |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unrevert of the revert #291, to reintroduce #286
--
Start using centralized TLS profile fetched from the APIServer configuration.
Remove kube-rbac-proxy