added code for wellknown kusto endpoints #456
Conversation
tanmaya-panda1
force-pushed
the
feature/AddWellKnownKustoEndpoint-clean
branch
from
58b1b73 to
70b72d3
Compare
tanmaya-panda1
had a problem deploying
to
build
GitHub Actions
Failure
| // Validate endpoint is trusted unless security checks are skipped | ||
| // Note: dmUrl might be empty/null in some test scenarios (e.g., mocked clients) | ||
| // Use @Suppress to handle potential platform nullability from Java interop | ||
| @Suppress("SENSELESS_COMPARISON") |
There was a problem hiding this comment.
Why is this annotation required ?
| */ | ||
| @JvmStatic | ||
| @JvmOverloads | ||
| fun validateTrustedEndpoint( |
There was a problem hiding this comment.
Why is JvmOverloads needed here. Both comments applicable on line 211. Lets check if we are missing some context before using the annotation
| * @param loginEndpoint The login endpoint to check against (optional, defaults to public cloud) | ||
| * @throws KustoClientInvalidConnectionStringException if endpoint is not trusted | ||
| */ | ||
| @JvmStatic |
There was a problem hiding this comment.
JvmStatic is used with companions to make the singleton method visible with the class. This is an object, is this annotation really needed
| * @param loginEndpoint The login endpoint to check against | ||
| * @throws KustoClientInvalidConnectionStringException if endpoint is not trusted | ||
| */ | ||
| @JvmStatic |
There was a problem hiding this comment.
Same comment as above here
| /** | ||
| * Data class representing the structure of WellKnownKustoEndpoints.json | ||
| */ | ||
| @Serializable |
There was a problem hiding this comment.
In the generated classes we import Kotlin serializable to an alias KSerializable and use it in the annotation to avoid confusion with Java Serializable. please evaluate if that may be a good idea
|
|
||
| constructor(message: String, cause: Throwable) : super(message, cause) | ||
|
|
||
| constructor(cause: Throwable) : super(cause) |
There was a problem hiding this comment.
Since both these calls just do super, are these constructors needed. Can we live with only the constructor below
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-resources-plugin</artifactId> | ||
| <version>3.3.1</version> |
There was a problem hiding this comment.
Nit: Reference as a property
This pull request introduces a new mechanism for validating that Kusto endpoints are trusted, using a configurable set of rules loaded from a shared JSON file. The validation logic is enforced at client initialization, and is extensible for custom or additional trusted hosts. The changes include new utility classes for endpoint rule matching, data loading from JSON, and a custom exception for invalid endpoints. The build process is also updated to ensure the JSON file is available at runtime.
Endpoint trust validation and configuration:
KustoTrustedEndpointsutility to validate Kusto endpoints against a set of trusted hostnames and suffixes, loaded fromWellKnownKustoEndpoints.json. The logic allows for overrides and additional trusted hosts, and throws a custom exception if validation fails.FastSuffixMatcherfor efficient hostname/suffix rule matching, and supporting data classes for rule and match result representation.WellKnownKustoEndpointsDatafor loading and parsing the trusted endpoints JSON file from the classpath, with robust error handling.KustoClientInvalidConnectionStringExceptionfor signaling invalid or untrusted endpoints.Integration and enforcement in client initialization:
KustoBaseApiClientto validate thedmUrlendpoint on initialization unless security checks are explicitly skipped, using the new trusted endpoints logic. [1] [2]Build process update:
ingest-v2/pom.xmlto copyWellKnownKustoEndpoints.jsonfrom the data module into the build output, ensuring the trusted endpoints configuration is always available at runtime.### AddedChanged
Fixed