Skip to content

[#3699] Add client ip to downstream message properties#3737

Open
jpforevers wants to merge 5 commits intoeclipse-hono:masterfrom
jpforevers:feature/downstream-properties-add-client-ip
Open

[#3699] Add client ip to downstream message properties#3737
jpforevers wants to merge 5 commits intoeclipse-hono:masterfrom
jpforevers:feature/downstream-properties-add-client-ip

Conversation

@jpforevers
Copy link

  1. Add configurable client_ip property for telemetry/event downstream messages across HTTP, MQTT, AMQP, CoAP, Lora, and Sigfox adapters.
  2. Support tenant and adapter overrides plus source strategy (auto, http-headers, proxy-protocol, remote-address) with Proxy Protocol enablement where applicable.
  3. Extend integration tests and update API/tenant/adapter documentation to describe new property and configuration.

Copy link
Contributor

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR 👍

This looks like a great start. I would like to propose a few changes around the configuration properties.

At the moment you seem to expect the tenant specific configuration to be found in the adapters/ext property of the TenantObject. However, the ext property is meant to be used for custom extensions only, i.e. for properties that (standard) Hono does not know about. In this case, however, we should make support for passing along the device IP a first-class citizen. I therefore suggest that you define the corresponding property names (using kebap-case) in RegistryManagementConstants and define corresponding properties in the Adapter class, e.g.

class Adapter {
  ...
  @JsonProperty(RegistryManagementConstants.FIELD_ADAPTERS_CLIENTIP_ENABLED)
  private boolean clientIpEnabled = false;
  ...
  public final boolean isClientIpEnabled() {
        return clientIpEnabled;
    }
}

and analogously for the clientIpSource config property.

You then also need to adapt the Device Registry Management API definition accordingly, to allow setting the properties when creating/updating a tenant.

Based on that, your code can then explicitly invoke TenantObject.getAdapter("http").isClientIpEnabled()

As for the protocol adapter configuration level, we use camel-case for the corresponding system properties. I would suggest to define

interface ProtocolAdapterOptions {
  @WithDefault("false")
  boolean clientIpEnabled();

  @WithDefault("auto")
  ClientIpSource clientIpSource();
}

The names of the environment variables would then be:

HONO_AMQP_CLIENTIPENABLED
HONO_AMQP_CLIENTIPSOURCE

and the system properties would be

hono.amqp.clientIpEnabled
hono.amqp.clientIpSource

If you want to group/encapsulate them, then I would suggest something like

interface ProtocolAdapterOptions {

  ClientIpOptions clientIp();

  interface ClientIpOptions {
    @WithDefault("false")
    boolean enabled();

    @WithDefault("auto")
    ClientIpSource source();
  }
}

The names of the environment variables would then be:

HONO_AMQP_CLIENTIP_ENABLED
HONO_AMQP_CLIENTIP_SOURCE

and the system properties would be

hono.amqp.clientIp.enabled
hono.amqp.clientIp.source

@jpforevers
Copy link
Author

Thank you for the feedback. The updates have been completed according to the suggestions:

  • The tenant adapter configuration has been promoted to first-class fields: client-ip-enabled / client-ip-source, and the Adapter model as well as device-registry-v1.yaml have been updated. The ext field is no longer used.

  • Global adapter configuration has been changed to nested camelCase: clientIp.enabled / clientIp.source (documentation has been updated to reflect environment variable and system property naming).

  • Documentation adjustments: removed client_ip from the Empty Notification table; added a textual description of client_ip in IPv4/IPv6 format (RFC 791/5952).

  • Tenant configuration in test cases has been changed to the adapter config approach.

Test results:

  • Command: mvn verify -Prun-tests -Dcoap.port=5683 -Dcoaps.port=5684 -Dcoap.ip=127.0.0.1

  • Result: Tests run: 552, Failures: 0, Errors: 0, Skipped: 9

jpforevers

This comment was marked as duplicate.

@jpforevers
Copy link
Author

jpforevers commented Feb 18, 2026

Thanks for the review! I’ve applied all requested changes (clientIpEnabled naming, removed redundant getter, moved ClientIpSource to core with @JsonProperty mappings, and updated usages/docs/tests). CI is green. Could you please take another look and re-review?

@jpforevers
Copy link
Author

Updated docs to clarify client-ip-source behavior across strategies (auto/http-headers/proxy-protocol/remote-address), including fallback rules and non-HTTP behavior. Commit: 34b0178.

@jpforevers
Copy link
Author

Follow-up: I clarified the client-ip-source behavior across all strategies in the docs and pushed the update (commit 34b0178). When you have a moment, could you please re-review?

@sophokles73
Copy link
Contributor

Hey, I am currently at a conference and won't be able to review for the next few days. I hope to find the time later during the week. Thanks for your patience 🙏

@jpforevers
Copy link
Author

Applied the three latest suggestions: updated the mqtt admin guide client-ip-enabled description, clarified device-registry client-ip-enabled wording, and added assertions for client_ip absence when not enabled (MQTT/AMQP/CoAP/Lora) with a new DownstreamMessageAssertions helper. Commit: 1bb2b14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants