jfrog / charts

JFrog official Helm Charts
https://jfrog.com/integration/helm-repository/
Apache License 2.0
254 stars 439 forks source link

Use POST request for federation livenessProbe #1847

Closed NiklasRosenstein closed 9 months ago

NiklasRosenstein commented 9 months ago

PR Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

What this PR does / why we need it:

It appears the /rtfs/sync/ping endpoint must be hit with a POST request instead of a GET request.

2023-12-07T15:11:24.336Z [jfrtfs] [ERROR] [               ] [.r.r.GlobalExceptionHandler:27] [http-nio-8085-exec-7] - Caught an error - Request method 'GET' is not supported
$ curl localhost:8085/rtfs/sync/ping
{"status":400,"message":"Request method 'GET' is not supported","traceId":"traceId"}

$ curl -XPOST localhost:8085/rtfs/sync/ping
{"status":200,"message":"PONG","traceId":"Take trace id"}

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

github-actions[bot] commented 9 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

NiklasRosenstein commented 9 months ago

I have read the CLA Document and I hereby sign the CLA

NiklasRosenstein commented 9 months ago

recheck

NiklasRosenstein commented 9 months ago

Seems this is also needed for startupProbe, and the path appears incorrect as well (/artifactory/system/rtfs/ping instead of /rtfs/ping)

eldada commented 9 months ago

Hi @NiklasRosenstein . Thank you for this PR, but this service is actually not production ready and should not even be enabled. The current federation functionality still runs in Artifactory. Simply disable this service (federation.enabled=false) and ignore it. The probes will be replaced with formal liveness and readiness as in other services when this service comes out officially. I'm closing this PR for now. Let us know if you require any further assistance.

NiklasRosenstein commented 9 months ago

Gotcha, thanks @eldada! I'm early into getting Artifactory set up and mistakenly assumed this service needs to run for Access Federation to work, although I had not tried it without yet.