bitflicker64 commented on code in PR #3349:
URL: https://github.com/apache/hugegraph/pull/3349#discussion_r4286278648
##########
helm/hugegraph/templates/tests/test-connection.yaml:
##########
@@ -78,7 +90,12 @@ spec:
PORT="{{ .Values.server.port }}"
FLOOR="{{ include "hugegraph.server.replicaFloor" . }}"
DEADLINE_S="${HG_TEST_DEADLINE_S:-150}"
- BODY={{ dict "gremlin" "graph.traversal().V().limit(1).count()"
"aliases" (dict "graph" (.Values.server.testGraphAlias | default
"DEFAULT-hugegraph")) | toJson | squote }}
+ BODY='{{ dict "gremlin" $smoke "aliases" (dict "graph"
.Values.server.testGraphAlias) | toJson | replace "'" "'\"'\"'" }}'
Review Comment:
Important: The merge with master dropped the `| default "DEFAULT-hugegraph"`
fallback that #3322 put on this alias, so a release whose stored values lack
`server.testGraphAlias` sends `"aliases":{"graph":null}` in both the
index-state and business requests.
`helm upgrade --reuse-values` sets the chart values to the old release's
coalesced values, so new chart defaults are not applied and a release installed
before d90946df1 has no `testGraphAlias`. That is the command
`docs/operations.md` gives for `--set-file server.testGremlin`. Rendering this
template with the 4b77b7bcb values.yaml gives `"graph":null` here and
`"graph":"DEFAULT-hugegraph"` on master. Gremlin Server reads the alias with
`asText()`, gets the string `null` and fails with `Could not rebind [graph] to
[null]`, so every Pod fails until the deadline. `server.testGraphAlias=null`
gives the same result.
Restore `(.Values.server.testGraphAlias | default "DEFAULT-hugegraph")` on
lines 93 and 95, and add a test with the key unset.
##########
.github/workflows/helm-chart-ci.yml:
##########
@@ -349,6 +349,19 @@ jobs:
expect "$(printf 'review\tsecret')" 'must be printable ASCII'
expect 'café' 'must be printable ASCII'
+ - name: helm test query assertions and shell quoting
+ run: |
+ # Execute the rendered query with the same Groovy version as Server.
+ GROOVY_VERSION=$(python3 - <<'PY_VERSION'
+ import xml.etree.ElementTree as ET
+ pom = ET.parse('pom.xml')
+ print(pom.find('{*}properties/{*}groovy.version').text)
+ PY_VERSION
+ )
+ curl -fsSLo /tmp/groovy.jar \
Review Comment:
Minor: The Groovy jar is downloaded and then executed with `java -cp`
without any checksum check, while the Helm tarball in the same job is pinned by
sha256.
A changed or substituted artifact at that URL would run in CI without being
noticed. The Groovy version comes from the POM, so a fixed hash cannot be
hardcoded, but Maven Central publishes `groovy-${GROOVY_VERSION}.jar.sha1` (and
`.asc`) next to the jar.
Fetch the `.sha1` file and check it with `sha1sum -c` before running the
fixtures.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]