[Remote clusters] Fix serialization for server_name field #60953
Conversation
alisonelizabeth
added
Feature:CCR and Remote Clusters
v8.0.0
Team:Kibana Management
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
There was a problem hiding this comment.
Hey @alisonelizabeth !
Thanks for working on this fix so quickly!!
I left a comment on the current approach, let me know what you think!
| @@ -157,7 +157,7 @@ export function serializeCluster(deserializedClusterObject: Cluster): ClusterPay | |||
| mode: mode ?? null, | |||
| proxy_address: proxyAddress ?? null, | |||
| proxy_socket_connections: proxySocketConnections ?? null, | |||
| server_name: serverName ?? null, | |||
| server_name: serverName && serverName !== '' ? serverName : null, | |||
There was a problem hiding this comment.
This part: && serverName !== '' will not be evaluated if serverName is '' because '' is already falsey. It looks like the fix is serverName ? serverName : null where the ?? was too restrictively looking at only overriding null or undefined.
Do you we face a similar challenge with the other values here?
There was a problem hiding this comment.
Or just serverName || null
There was a problem hiding this comment.
🤦♀ you're correct! I updated the code. I think it makes sense to use <value> || null for all values as any falsely value would be invalid.
|
@jloleysens I updated this PR based on your feedback. Great catch! Mind taking another look? |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Fixes #60902
This PR addresses the scenario where a user has a remote cluster with the
server_namefield defined, and then decides to remove the value via the edit flow.Note: This fixes the UI logic, but it appears there is also a bug on the ES side. See: elastic/elasticsearch#53634 (comment)
How to test
Server namedefinedServer namevalue (empty input)GET _cluster/settingsand check that theserver_namefield is no longer associated with the remote cluster.