Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add nucleus connectivity validation #1587

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tiancishen
Copy link
Contributor

@tiancishen tiancishen commented Mar 12, 2024

Issue #, if available:

Description of changes:

  • Add validations to check mqtt and http connections work after merging in Nucleus configuration
  • Validation can be disabled if the component validation timeout is set to 0, the validateConnectivityDuringDeployment Nucleus config is set to false, or the device is configured to be offline

Example of errors that can be seen on the console:

MQTT validation failures

  • NUCLEUS_CONNECTIVITY_CONFIG_NOT_VALID: FAILED_NO_STATE_CHANGE: Mqtt client failed to connect with the new configuration: port must be a positive integer between 1 and 65535
  • NUCLEUS_CONNECTIVITY_CONFIG_NOT_VALID: FAILED_NO_STATE_CHANGE: Mqtt client validation timed out with the new configuration
  • NUCLEUS_CONNECTIVITY_CONFIG_NOT_VALID: FAILED_NO_STATE_CHANGE: Mqtt client validation completed exceptionally with the new configuration: software.amazon.awssdk.crt.mqtt.MqttException: Host name was invalid for dns resolution

HTTP validation failure

  • NUCLEUS_CONNECTIVITY_CONFIG_NOT_VALID: FAILED_NO_STATE_CHANGE: HTTP client validation with the new configuration Received an UnknownHostException when attempting to interact with a service. See cause for the exact endpoint that is failing to resolve. If this is happening on an endpoint that previously worked, there may be a network connectivity issue or your DNS cache could be storing endpoints for too long.

Why is this change necessary:
Nucleus doesn't validate mqtt and http connections when merging in a new configuration from deployment. Prevent Nucleus from accepting a bad configuration deployment by validating connectivity with the deploymeny Nucleus configuration

How was this change tested:

  • Updated or added new unit tests.
  • Updated or added new integration tests.
  • Updated or added new end-to-end tests.
  • If my code makes a remote network call, it was tested with a proxy.

Test using UATs covers the following scenarios:

  • Bad network Proxy
  • Bad Mqtt port
  • Bad Mqtt timeouts
  • Bad Iot data endpoint
  • Bad GG data endpoint and port
  • Bad config with validation disabled

Any additional information or context required to review the change:

Documentation Checklist:

  • Updated the README if applicable.

Compatibility Checklist:

  • I confirm that the change is backwards compatible.
  • Any modification or deletion of public interfaces does not impact other plugin components.
  • For external library version updates, I have reviewed its change logs and Nucleus does not consume
    any deprecated method or type.

Refer to Compatibility Guidelines for more information.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@tiancishen tiancishen marked this pull request as draft March 12, 2024 20:17
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch 2 times, most recently from 60934b7 to 5446985 Compare March 13, 2024 01:19
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from 5446985 to 813116b Compare March 18, 2024 17:41
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from 813116b to 4de2fb2 Compare March 19, 2024 17:40
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch 3 times, most recently from 8f9f232 to 01b13f1 Compare March 19, 2024 21:42
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from 01b13f1 to 027aacf Compare March 19, 2024 21:58
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch 4 times, most recently from 6a50d3d to f32b2d5 Compare May 9, 2024 23:40
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from f32b2d5 to 7201e9f Compare May 10, 2024 18:59
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from 7201e9f to aacbfdf Compare May 14, 2024 17:46
alter-mage
alter-mage previously approved these changes May 14, 2024
@tiancishen tiancishen requested a review from MikeDombo May 14, 2024 20:15
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from 875fa60 to c63d7af Compare May 17, 2024 00:49
@tiancishen tiancishen force-pushed the resilientNucleusConfig branch from c63d7af to 0c21629 Compare May 17, 2024 01:17
@MikeDombo MikeDombo dismissed their stale review May 17, 2024 18:15

updated

@junfuchen99 junfuchen99 added the work-in-progress PR is actively being worked on and should not be considered ready for review label Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress PR is actively being worked on and should not be considered ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants