Skip to content

Commit

Permalink
Corrects CloudDNS implementation as it doesn't support duplicate zones
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrian Cole committed Mar 21, 2015
1 parent 98f1440 commit 394536d
Show file tree
Hide file tree
Showing 11 changed files with 153 additions and 88 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Publishes model and core test jars
* Adds example server
* Enforces source compatibility with animal-sniffer
* Corrects Designate implementation as it doesn't support duplicate zones
* Corrects CloudDNS and Designate implementations as they don't support duplicate zones

### Version 4.4.2
* Updates to feign 8.1
Expand Down
4 changes: 2 additions & 2 deletions cli/src/test/java/denominator/cli/DenominatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public void listsAllProvidersWithCredentials() {
assertThat(ListProviders.providerAndCredentialsTable())
.isEqualTo(Util.join('\n',
"provider url duplicateZones credentialType credentialArgs",
"clouddns https://identity.api.rackspacecloud.com/v2.0 true password username password",
"clouddns https://identity.api.rackspacecloud.com/v2.0 true apiKey username apiKey",
"clouddns https://identity.api.rackspacecloud.com/v2.0 false password username password",
"clouddns https://identity.api.rackspacecloud.com/v2.0 false apiKey username apiKey",
"designate http://localhost:5000/v2.0 false password tenantId username password",
"dynect https://api2.dynect.net/REST false password customer username password",
"mock mem:mock false ",
Expand Down
27 changes: 25 additions & 2 deletions clouddns/src/main/java/denominator/clouddns/CloudDNSFunctions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import java.util.List;
import java.util.Map;

import denominator.clouddns.RackspaceApis.CloudDNS;
import denominator.clouddns.RackspaceApis.Job;
import denominator.clouddns.RackspaceApis.Record;
import denominator.model.rdata.AAAAData;
import denominator.model.rdata.AData;
Expand All @@ -11,12 +13,30 @@
import denominator.model.rdata.NSData;
import denominator.model.rdata.SRVData;
import denominator.model.rdata.TXTData;
import feign.RetryableException;
import feign.Retryer;

import static denominator.common.Util.split;
import static java.lang.String.format;

public final class CloudDNSFunctions {
final class CloudDNSFunctions {

private CloudDNSFunctions() {
static void awaitComplete(CloudDNS api, Job job) {
RetryableException retryableException = new RetryableException(
format("Job %s did not complete. Check your logs.", job.id), null);
Retryer retryer = new Retryer.Default(500, 1000, 30);

while (true) {
job = api.getStatus(job.id);

if ("COMPLETED".equals(job.status)) {
break;
} else if ("ERROR".equals(job.status)) {
throw retryableException;
}

retryer.continueOrPropagate(retryableException);
}
}

static Map<String, Object> toRDataMap(Record record) {
Expand All @@ -43,4 +63,7 @@ static Map<String, Object> toRDataMap(Record record) {
throw new UnsupportedOperationException("record type not yet supported" + record);
}
}

private CloudDNSFunctions() {
}
}
9 changes: 3 additions & 6 deletions clouddns/src/main/java/denominator/clouddns/CloudDNSProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import denominator.DNSApiManager;
import denominator.ResourceRecordSetApi;
import denominator.ZoneApi;
import denominator.clouddns.RackspaceAdapters.DomainIdListAdapter;
import denominator.clouddns.RackspaceAdapters.DomainListAdapter;
import denominator.clouddns.RackspaceAdapters.JobIdAndStatusAdapter;
import denominator.clouddns.RackspaceAdapters.RecordListAdapter;
Expand Down Expand Up @@ -70,11 +71,6 @@ public Map<String, Collection<String>> profileToRecordTypes() {
return profileToRecordTypes;
}

@Override
public boolean supportsDuplicateZoneNames() {
return true;
}

@Override
public Map<String, Collection<String>> credentialTypeToParameterNames() {
Map<String, Collection<String>> options = new LinkedHashMap<String, Collection<String>>();
Expand Down Expand Up @@ -109,7 +105,7 @@ ResourceRecordSetApi.Factory provideResourceRecordSetApiFactory(
}

@dagger.Module(injects = CloudDNSResourceRecordSetApi.Factory.class,
complete = false // doesn't bind Provider used by DesignateTarget
complete = false // doesn't bind Provider used by CloudDNSTarget
)
public static final class FeignModule {

Expand Down Expand Up @@ -146,6 +142,7 @@ Feign feign(Logger logger, Logger.Level logLevel) {
new KeystoneAccessAdapter("rax:dns"),
new JobIdAndStatusAdapter(),
new DomainListAdapter(),
new DomainIdListAdapter(),
new RecordListAdapter()))
)
.build();
Expand Down
39 changes: 11 additions & 28 deletions clouddns/src/main/java/denominator/clouddns/CloudDNSResourceRecordSetApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@

import denominator.ResourceRecordSetApi;
import denominator.clouddns.RackspaceApis.CloudDNS;
import denominator.clouddns.RackspaceApis.JobIdAndStatus;
import denominator.clouddns.RackspaceApis.ListWithNext;
import denominator.clouddns.RackspaceApis.Pager;
import denominator.clouddns.RackspaceApis.Record;
import denominator.common.Util;
import denominator.model.ResourceRecordSet;
import feign.RetryableException;
import feign.Retryer;

import static denominator.clouddns.CloudDNSFunctions.awaitComplete;
import static denominator.clouddns.CloudDNSFunctions.toRDataMap;
import static denominator.clouddns.RackspaceApis.emptyOn404;
import static denominator.common.Preconditions.checkArgument;
Expand Down Expand Up @@ -89,10 +87,10 @@ public void put(ResourceRecordSet<?> rrset) {
continue;
}

awaitComplete(api.updateRecord(domainId, record.id, rrset.ttl(), record.data()));
awaitComplete(api, api.updateRecord(domainId, record.id, rrset.ttl(), record.data()));
}
} else {
awaitComplete(api.deleteRecord(domainId, record.id));
awaitComplete(api, api.deleteRecord(domainId, record.id));
}
}

Expand All @@ -104,32 +102,15 @@ public void put(ResourceRecordSet<?> rrset) {
String data = join(' ', mutableRData.values().toArray());

if (priority == null) {
awaitComplete(api.createRecord(domainId, rrset.name(), rrset.type(), ttlToApply, data));
awaitComplete(api,
api.createRecord(domainId, rrset.name(), rrset.type(), ttlToApply, data));
} else {
awaitComplete(api.createRecordWithPriority(
awaitComplete(api, api.createRecordWithPriority(
domainId, rrset.name(), rrset.type(), ttlToApply, data, priority));
}
}
}

private void awaitComplete(JobIdAndStatus job) {
RetryableException retryableException = new RetryableException(
String.format("JobId %s did not complete. Check your logs.", job.id), null);
Retryer retryer = new Retryer.Default(500, 1000, 30);

while (true) {
JobIdAndStatus jobIdAndStatus = api.getStatus(job.id);

if ("COMPLETED".equals(jobIdAndStatus.status)) {
break;
} else if ("ERROR".equals(jobIdAndStatus.status)) {
throw retryableException;
}

retryer.continueOrPropagate(retryableException);
}
}

/**
* Has the side effect of removing the priority from the mutableRData.
*
Expand All @@ -153,7 +134,7 @@ public void deleteByNameAndType(String name, String type) {
checkNotNull(type, "type");

for (Record record : api.recordsByNameAndType(domainId, name, type)) {
awaitComplete(api.deleteRecord(domainId, record.id));
awaitComplete(api, api.deleteRecord(domainId, record.id));
}
}

Expand Down Expand Up @@ -200,8 +181,10 @@ static final class Factory implements denominator.ResourceRecordSetApi.Factory {
}

@Override
public ResourceRecordSetApi create(String id) {
return new CloudDNSResourceRecordSetApi(api, Integer.parseInt(id));
public ResourceRecordSetApi create(String name) {
ListWithNext<Integer> matching = api.domainIdsByName(name);
checkArgument(!matching.isEmpty(), "zone %s does not exist", name);
return new CloudDNSResourceRecordSetApi(api, matching.get(0));
}
}
}
45 changes: 33 additions & 12 deletions clouddns/src/main/java/denominator/clouddns/RackspaceAdapters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.util.Collections;
import java.util.Comparator;

import denominator.clouddns.RackspaceApis.JobIdAndStatus;
import denominator.clouddns.RackspaceApis.Job;
import denominator.clouddns.RackspaceApis.ListWithNext;
import denominator.clouddns.RackspaceApis.Record;
import denominator.model.Zone;
Expand All @@ -28,36 +28,60 @@ private static <X> Comparator<X> toStringComparator() {
return Comparator.class.cast(TO_STRING_COMPARATOR);
}

static class JobIdAndStatusAdapter extends TypeAdapter<JobIdAndStatus> {
static class JobIdAndStatusAdapter extends TypeAdapter<Job> {

@Override
public void write(JsonWriter out, JobIdAndStatus value) throws IOException {
public void write(JsonWriter out, Job value) throws IOException {
// never need to write this object
}

@Override
public JobIdAndStatus read(JsonReader reader) throws IOException {
JobIdAndStatus jobIdAndStatus = new JobIdAndStatus();
public Job read(JsonReader reader) throws IOException {
Job job = new Job();

reader.beginObject();

while (reader.hasNext()) {
String key = reader.nextName();
if (key.equals("jobId")) {
jobIdAndStatus.id = reader.nextString();
job.id = reader.nextString();
} else if (key.equals("status")) {
jobIdAndStatus.status = reader.nextString();
job.status = reader.nextString();
} else {
reader.skipValue();
}
}

reader.endObject();

return jobIdAndStatus;
return job;
}
}

static class DomainIdListAdapter extends ListWithNextAdapter<Integer> {

@Override
protected String jsonKey() {
return "domains";
}

protected Integer build(JsonReader reader) throws IOException {
Integer id = null;
while (reader.hasNext()) {
if (reader.nextName().equals("id")) {
id = reader.nextInt();
} else {
reader.skipValue();
}
}
return id;
}
}

/**
* Note that we do not expose the ID back to the user as ID isn't semantic when the API doesn't
* support multiple zones
*/
static class DomainListAdapter extends ListWithNextAdapter<Zone> {

@Override
Expand All @@ -69,11 +93,8 @@ protected Zone build(JsonReader reader) throws IOException {
String name = null;
String id = null;
while (reader.hasNext()) {
String key = reader.nextName();
if (key.equals("name")) {
if (reader.nextName().equals("name")) {
name = reader.nextString();
} else if (key.equals("id")) {
id = reader.nextString();
} else {
reader.skipValue();
}
Expand Down
31 changes: 18 additions & 13 deletions clouddns/src/main/java/denominator/clouddns/RackspaceApis.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,14 @@ static interface CloudDNS {
Map<String, Object> limits();

@RequestLine("GET /status/{jobId}?showDetails=true")
JobIdAndStatus getStatus(@Param("jobId") String jobId);
Job getStatus(@Param("jobId") String jobId);

/**
* Note this doesn't make sense to return anything except one or none, as duplicate domains
* aren't permitted in the create api.
*/
@RequestLine("GET /domains?name={name}")
ListWithNext<Integer> domainIdsByName(@Param("name") String name);

@RequestLine("GET")
ListWithNext<Zone> domains(URI href);
Expand All @@ -66,27 +73,25 @@ ListWithNext<Record> recordsByNameAndType(@Param("domainId") int id,
@RequestLine("POST /domains/{domainId}/records")
@Body("%7B\"records\":[%7B\"name\":\"{name}\",\"type\":\"{type}\",\"ttl\":\"{ttl}\",\"data\":\"{data}\"%7D]%7D")
@Headers("Content-Type: application/json")
JobIdAndStatus createRecord(@Param("domainId") int id, @Param("name") String name,
@Param("type") String type, @Param("ttl") int ttl,
@Param("data") String data);
Job createRecord(@Param("domainId") int id, @Param("name") String name,
@Param("type") String type, @Param("ttl") int ttl, @Param("data") String data);

@RequestLine("POST /domains/{domainId}/records")
@Body("%7B\"records\":[%7B\"name\":\"{name}\",\"type\":\"{type}\",\"ttl\":\"{ttl}\",\"data\":\"{data}\",\"priority\":\"{priority}\"%7D]%7D")
@Headers("Content-Type: application/json")
JobIdAndStatus createRecordWithPriority(@Param("domainId") int id, @Param("name") String name,
@Param("type") String type, @Param("ttl") int ttl,
@Param("data") String data,
@Param("priority") int priority);
Job createRecordWithPriority(@Param("domainId") int id, @Param("name") String name,
@Param("type") String type, @Param("ttl") int ttl,
@Param("data") String data, @Param("priority") int priority);

@RequestLine("PUT /domains/{domainId}/records/{recordId}")
@Body("%7B\"ttl\":\"{ttl}\",\"data\":\"{data}\"%7D")
@Headers("Content-Type: application/json")
JobIdAndStatus updateRecord(@Param("domainId") int domainId, @Param("recordId") String recordId,
@Param("ttl") int ttl, @Param("data") String data);
Job updateRecord(@Param("domainId") int domainId, @Param("recordId") String recordId,
@Param("ttl") int ttl, @Param("data") String data);

@RequestLine("DELETE /domains/{domainId}/records/{recordId}")
JobIdAndStatus deleteRecord(@Param("domainId") int domainId,
@Param("recordId") String recordId);
Job deleteRecord(@Param("domainId") int domainId,
@Param("recordId") String recordId);
}

interface Pager<X> {
Expand All @@ -100,7 +105,7 @@ static class TokenIdAndPublicURL {
String publicURL;
}

static class JobIdAndStatus {
static class Job {

String id;
String status;
Expand Down
3 changes: 1 addition & 2 deletions clouddns/src/test/java/denominator/clouddns/CloudDNSProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static denominator.Providers.list;
import static denominator.Providers.provide;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertTrue;

public class CloudDNSProviderTest {

Expand All @@ -30,7 +29,7 @@ public class CloudDNSProviderTest {
@Test
public void testCloudDNSMetadata() {
assertThat(PROVIDER.name()).isEqualTo("clouddns");
assertTrue(PROVIDER.supportsDuplicateZoneNames());
assertThat(PROVIDER.supportsDuplicateZoneNames()).isFalse();
assertThat(PROVIDER.credentialTypeToParameterNames())
.containsEntry("password", Arrays.asList("username", "password"))
.containsEntry("apiKey", Arrays.asList("username", "apiKey"));
Expand Down
Loading

0 comments on commit 394536d

Please sign in to comment.