nosinovacao / name-sdk Goto Github PK

AppVeyor and Travis-CI should detect the build warnings and somehow fail the build if any is encountered.

For AppVeyor, a good starting point may be to use the provided logger.

By @suvl at nosinovacao/name-registry-api#1

A field should be added to the manifest providing insight on the error level of the last announced manifest. That field should have three levels, as suggested (not final):

• OK: no errors
• WARN: the current errors won't let NAME know if there's anything wrong or not (dependencies do not have NAME installed or NAME version is invalid)
• ERR: all other errors not included in WARN

With Asp.Net Core, the standard behaviour for managing settings is with the use of the IConfigurationProvider.

The dependencies.json should support a new connectionstring Locator IConfigurationProvider which takes accepts the key element containing the name of the setting as described in the microsoft documentation (i.e. section:subsection:key).

The IConfigurationProvider class is in the Microsoft.Extensions.Configuration.Abstractions nuget package, since we don't want to add this dependency in the main NAME package we need to add an extension point in the DependenciesReader.ReadDependencies method that allows the caller to inject custom IConnectionStringProviders.

Furthermore NAMEAspNetCoreConfiguration should optionally take in the IConfigurationProvider and use the new extension point.

The explore dependencies feature appears to be ill-designed and is making unnecessary requests forward.

Review ServiceVersionResolver.cs

Add a version resolver for Kafka service, not depending on Kafka driver (only protocol network level).

In some cases, when fetching a dependency manifest, said dependency may return a 200 status code even if NAME is not listening in that endpoint. Currently NAME fails to deserialize the manifest from the response and fails for that dependency.

Changes:

Before deserializing the manifest, NAME should look at the returned X-NAME-Manifest-Endpoint header and check if the requested endpoint was the same as the header, if true, it should deserialize the manifest. Otherwise it should abort and try to get the manifest from the correct endpoint.

This allows use cases where the maximum version of a dependency is specified by just one portion of the version.

Example:

Dependency A depends on the major version 3 of Dependency B, regardless of the minor version.

Current Behaviour:

One would have to set the minimum version to 3 and maximum version to 3.99999.99999 which is far from ideal.

Expected Behaviour:

Ideally, one could set the minimum version 3 and maximum version to 3.*, which would match any version with the major portion 3.

As is, the port in which NAME is listening is sent to the Registry as null when using the WebApi and AspNetCore packages.

Getting the port is not trivial, and some discussion may be needed.

For apps hosted in IIS we can use the Microsoft.Web.Administration namespace to get this information, so at least the NAME.WebApi package should send the port.

There are scenarios where connection strings are set using environment variables (e.g. docker), that scenarios should be supported.

Changes:

Add a new EnvironmentConnectionStringLocator class and change the dependencies file parser to read the locator, which only receives the environment variable name.

Currently, when NAME is not installed on the target service dependency, the manifest outputs the message "Dependency Service is not reachable", this should be changed to return a relevant message, such as "NAME is not installed."

To do this, NAME should check if the header X-NAME-Manifest-Location is present when a service dependency check fails.

if something is not correctly configured to work properly the warning messages should help to solve the issue.

In this case it should describe or point to some urls where an explanation is presented.

[WARN] Could not register with http://registry1.name.local.internal:80/api/v1: An error occurred while sending the request. The server name or address could not be resolved.

Add a version resolver for Redis cache.

Just like the other version resolvers we can't depend on any redis client implementation, as such we must use the REdis Serialization Protocol to identify the version used by the server.

As of now, the startup healthcheck is always synchronous. As a result, when there are unreachable dependencies and/or dependencies without NAME installed (or any other unresponsive situation from the dependencies) at various levels, the application might take up to 20 seconds to start (max. timeout for the startup healthcheck as of now).

When ThrowOnDependenciesFail is false, that wait is not necessary at all and is indeed detrimental to NAME acceptance and roll-out.

Changes

• If ThrowOnDependenciesFail = true, startup healthcheck should be synchronous and should throw as expected;
• If ThrowOnDependenciesFail = false, startup healthcheck will be asynchronous and should not throw any exception;
• An event will be created and invoked when an exception is raised, allowing for the developer to decide what to (i.e. if the dependency is quite critical to the app, raise some alert mechanism or even kill the app)

In some scenarios, the MongoDB version checker, cannot read correctly the BSON format for a specific datatype, in this case the timestamp format.

Error message: "[Error] Don’t know elementType=17"

MongoDB version: v3.4.15

DataTypes: https://docs.mongodb.com/manual/reference/operator/query/type/

Like it exists the "/manifest/ui" with a specific format to expose the manifest. It should also exist an endpoint like "/manifest/flowchart" to support a flowchart language to display the dependencies in a flow chart way.

For more info: https://mermaidjs.github.io/
For more info in the flow chart part: https://mermaidjs.github.io/flowchart.html

Currently, NAME does not support elasticsearch integration.

In order to obtain elasticsearch information, one should perform the following query:

# curl http://elasticsearch:9200
{
  "name" : "a5045f503f93",
  "cluster_name" : "use",
  "cluster_uuid" : "Czl7TeOvTEe0-8sXhiKUNw",
  "version" : {
    "number" : "5.5.1",
    "build_hash" : "19c13d0",
    "build_date" : "2017-07-18T20:44:24.823Z",
    "build_snapshot" : false,
    "lucene_version" : "6.6.0"
  },
  "tagline" : "You Know, for Search"
}

If you set a '\t' character before a comment in the dependencies.json file, the parser will fail to recognize the line as a comment and will fail to load the file.

Changes:

In the dependencies file parser, when removing the comments from the file, instead of checking for lines starting with ' ', all white characters (except '\n') should be checked.

When we have an external dependency that does not have NAME nor we can control the source code of that dependency it makes sense to allow a different dependency check. For instance, check for a simple HTTP GET an check the returned HTTP Status Code.

Proposal of change:
{ "name": "WebPA Tridium", "min_version": "1.0", "max_version": "1.*", "check_override_uri": "{connection_string}/health", "check_override_codes": "200;400;401", "connection_string": { "locator": "IConfiguration", "key": "WebPaHttpClient:BaseUrl" } }

New properties:

• check_override_uri
  The URI used to check if the dependency is up and running
• check_override_codes
  The accepted HTTP status codes (or that follow the same network like status code format) for the NAME to consider the dependency status as "Ok", otherwise it should be considered "Error"

When behind an SSL Termination proxy, NAME sets the url in the manifest ui HTML with the HTTP protocol instead of HTTPS. This causes the javascript request to fail.

Changes:

In order to resolve this and prevent future problems, the url to be set in the HTML should be relative instead of absolute.