Aias00 commented on code in PR #7286:
URL: https://github.com/apache/shenyu/pull/7286#discussion_r4109923512
##########
shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/parser/ContextPathParser.java:
##########
@@ -124,6 +140,54 @@ private List<IngressConfiguration> parseIngressRule(final
V1IngressRule ingressR
return res;
}
+ /**
+ * Resolve the context path annotations used to build the context mapping
rule. An RPC ingress
+ * declares them on the metadata service that is referenced by the ingress
labels, the same
+ * resource the RPC parsers read their annotations from, so the service
annotations are used
+ * when the ingress does not declare them itself.
+ *
+ * @param ingress ingress resource
+ * @return the resolved context path annotations, empty if neither the
ingress nor the
+ * referenced services declare them
+ */
+ private Map<String, String> resolveContextPathAnnotations(final V1Ingress
ingress) {
+ if (Objects.isNull(ingress.getMetadata())) {
+ return Collections.emptyMap();
+ }
+ V1ObjectMeta metadata = ingress.getMetadata();
+ Map<String, String> serviceAnnotations =
getReferencedServiceAnnotations(metadata.getNamespace(), metadata.getLabels());
+ Map<String, String> ingressAnnotations =
MapUtils.emptyIfNull(metadata.getAnnotations());
+ Map<String, String> res = new
HashMap<>(CONTEXT_PATH_ANNOTATION_KEYS.size());
+ for (String key : CONTEXT_PATH_ANNOTATION_KEYS) {
+ String value = StringUtils.isNotBlank(ingressAnnotations.get(key))
? ingressAnnotations.get(key) : serviceAnnotations.get(key);
+ if (Objects.nonNull(value)) {
+ res.put(key, value);
+ }
+ }
+ return res;
+ }
+
+ private Map<String, String> getReferencedServiceAnnotations(final String
namespace, final Map<String, String> labels) {
Review Comment:
Suggestion (non-blocking): this scans every label value as a service name.
GrpcParser#parseIngressRule does exactly the same today
(GrpcParser.java:255-256), so this is consistent rather than a new defect - but
it means a label such as app: shenyu-examples-grpc-service is consulted too,
and if that service declares plugin-context-path-path it silently becomes the
context path of an ingress that declares nothing itself. Second, when two
referenced services declare different values the winner depends on the HashMap
iteration order of the labels, so the result is not reproducible between
restarts. Suggest restricting the scan to the
shenyu.apache.org/metadata-labels-* keys and iterating them in a defined order,
or keeping parity here and fixing GrpcParser the same way in a follow-up so
both lookup paths stay aligned.
##########
shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/parser/ContextPathParser.java:
##########
@@ -124,6 +140,54 @@ private List<IngressConfiguration> parseIngressRule(final
V1IngressRule ingressR
return res;
}
+ /**
+ * Resolve the context path annotations used to build the context mapping
rule. An RPC ingress
+ * declares them on the metadata service that is referenced by the ingress
labels, the same
+ * resource the RPC parsers read their annotations from, so the service
annotations are used
+ * when the ingress does not declare them itself.
+ *
+ * @param ingress ingress resource
+ * @return the resolved context path annotations, empty if neither the
ingress nor the
+ * referenced services declare them
+ */
+ private Map<String, String> resolveContextPathAnnotations(final V1Ingress
ingress) {
+ if (Objects.isNull(ingress.getMetadata())) {
+ return Collections.emptyMap();
+ }
+ V1ObjectMeta metadata = ingress.getMetadata();
+ Map<String, String> serviceAnnotations =
getReferencedServiceAnnotations(metadata.getNamespace(), metadata.getLabels());
+ Map<String, String> ingressAnnotations =
MapUtils.emptyIfNull(metadata.getAnnotations());
+ Map<String, String> res = new
HashMap<>(CONTEXT_PATH_ANNOTATION_KEYS.size());
Review Comment:
Nit: new HashMap<>(CONTEXT_PATH_ANNOTATION_KEYS.size()) creates capacity 3,
i.e. a resize threshold of 2, so inserting all three keys resizes the map right
away. Same pattern with labels.size() a few lines below. Harmless, just a
slightly larger initial capacity would avoid it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
