Skip to content

Conversation

@Sokitha-21
Copy link
Member

  • Added endpoint builders and query helpers for health/status, zones, record sets, groups and users.
  • Implemented client methods for monitoring, status management, backend IDs, deleted zones, ACL rule add/delete, record set metrics and history, group utilities, and user administration.
  • Introduced ownership transfer helpers for record sets (request/approve/reject/cancel) that wrap record set update payloads.
  • Added unit tests covering all new endpoints and client methods.
  • Fix SigV4 signing for DELETE endpoint.

@snyk-io
Copy link

snyk-io bot commented Jan 21, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link
Contributor

@JoshSEdwards JoshSEdwards left a comment

Choose a reason for hiding this comment

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

Just a few minor Javadoc discrepancies and some suggestions to make error handling more consistent.

VinylDNSResponse<GetRecordSetResponse> rsResponse =
getRecordSet(new GetRecordSetRequest(payload.getZoneId(), payload.getRecordSetId()));
if (!(rsResponse instanceof VinylDNSSuccessResponse)) {
throw new RuntimeException("Failed to fetch RecordSet before transfer");
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be better to return a VinylDNSFailureResponse that includes the upstream status/body instead of throwing, to keep error handling consistent for callers.


http.setEntity(new ByteArrayEntity(payloadBytes, ContentType.APPLICATION_JSON));

try (CloseableHttpClient httpClient =
Copy link
Contributor

Choose a reason for hiding this comment

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

This DELETE‑with‑body branch builds a new Apache client on each call instead of using the reusable AmazonHttpClient created in the constructor (VinylDNSClientImpl.java:83). That can bypass any configured timeouts/retries/pooling and risks hanging or inconsistent behavior vs other requests.

VinylDNSResponse<StatusResponse> updateStatus(UpdateStatusRequest request);

/**
* Requests ownership transfer for a record seFt.
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo here -> record seFt

/**
* Retrieves the list of valid email domains for groups.
*
* @return {@link VinylDNSSuccessResponse VinylDNSSuccessResponse&lt;ListValidDomainsResponse&gt;}
Copy link
Contributor

Choose a reason for hiding this comment

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

listValidDomains() returns VinylDNSResponse<List>, but Javadoc says ListValidDomainsResponse

/**
* Lists available DNS backend IDs that can be used for zones.
*
* @return {@link VinylDNSSuccessResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

getZonesBackendIds() returns VinylDNSResponse<List>, but Javadoc says GetZonesBackendIdsResponse

/**
* Simple health check.
*
* @return {@link VinylDNSSuccessResponse VinylDNSSuccessResponse&lt;GetPingResponse&gt;} in case
Copy link
Contributor

Choose a reason for hiding this comment

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

getPing() returns VinylDNSResponse, but Javadoc says GetPingResponse

/**
* Comprehensive health check including subsystem statuses.
*
* @return {@link VinylDNSSuccessResponse VinylDNSSuccessResponse&lt;GetHealthResponse&gt;} in
Copy link
Contributor

Choose a reason for hiding this comment

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

getHealth() returns VinylDNSResponse, but Javadoc says GetHealthResponse

/**
* Returns current deployment color (blue/green).
*
* @return {@link VinylDNSSuccessResponse VinylDNSSuccessResponse&lt;GetColorResponse&gt;} in case
Copy link
Contributor

Choose a reason for hiding this comment

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

getColor() returns VinylDNSResponse, but Javadoc says GetColorResponse

/**
* Prometheus metrics in text/plain exposition format.
*
* @return {@link VinylDNSSuccessResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

getPrometheusMetrics() returns VinylDNSResponse, but Javadoc says GetPrometheusMetricsResponse

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