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

DnsValidatedCertificate reports DNS zone example.com to be non-authoritative #2076

Closed
alekitto opened this issue Mar 22, 2019 · 9 comments · Fixed by #2995 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5, MechanicalRock/cdk-constructs#6 or MechanicalRock/cdk-constructs#7

Comments

@alekitto
Copy link
Contributor

When requesting a DnsValidatedCertificate using an imported HostedZone (imported with HostedZoneProvider.findAndImport) not previously saved in cdk.context.json, validation fails with error DNS zone example.com is not authoritative for certificate domain name cdn.mydomain.com

Same error is present when requesting the certificate for the root domain name:

new DnsValidatedCertificate(stack, 'MyCertificate', {
    domainName: 'mydomain.com',
    hostedZone,
});

results in: DNS zone mydomain.com is not authoritative for certificate domain name mydomain.com

@troyready
Copy link

Anyone else experiencing this too? Seems like an issue with findAndImport specifically, works with a direct HostedZone.import

@jarikujansuu
Copy link

During weekend I tried project based on CDK examples and it uses that findAndImport (https://github.com/aws-samples/aws-cdk-examples/blob/master/typescript/static-site/static-site.ts) and had same problem

Have to see if import works

@waynerobinson
Copy link

I'm having exactly the same problem.

@zacharycarter
Copy link

I believe it's caused by this -

const DEFAULT_HOSTED_ZONE: HostedZoneContextResponse = {
  Id: '/hostedzone/DUMMY',
  Name: 'example.com',
};
  public findHostedZone(): HostedZoneAttributes {
    const zone = this.provider.getValue(DEFAULT_HOSTED_ZONE) as HostedZoneContextResponse;
    // CDK handles the '.' at the end, so remove it here
    if (zone.Name.endsWith('.')) {
      zone.Name = zone.Name.substring(0, zone.Name.length - 1);
    }
    return {
      hostedZoneId: zone.Id,
      zoneName: zone.Name,
    };
  }
}

https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-route53/lib/hosted-zone-provider.ts

Seems that it's always using the default.

@stefanorg
Copy link

I've the same problem, anyone found a solution?

@BlueYetii
Copy link

I've found the same issue while trying to create some certificates today. The workaround (although not pretty) was to create a separate stack that just imports the hosted zone and makes the imported zone available to pass to other stacks. Then running cdk synth -e HostedZoneStack generates the correct cdk.context.json allowing other stacks containing the certificates to be deployed.

Hosted Zone Stack

import cdk = require('@aws-cdk/cdk');
import route53 = require('@aws-cdk/aws-route53');

export interface HostZoneStackProps extends cdk.StackProps {
    /**
     * Root level domain for the hosted zone
     */
    domain: string;
}

export class HostZoneStack extends cdk.Stack {
    public hostedZone: route53.IHostedZone;

    constructor(scope: cdk.Construct, id: string, props: HostZoneStackProps) {
        super(scope, id, props);

        this.hostedZone = new route53.HostedZoneProvider(this, {
                domainName: props.domain,
                privateZone: false
        }).findAndImport(this, "HostedZone");
    }
}

Example App

#!/usr/bin/env node
import "source-map-support/register";
import cdk = require("@aws-cdk/cdk");
import { ApplicationStack } from "../lib/application-stack";
import { HostZoneStack } from "../lib/hosted-zone-stack";

const app = new cdk.App();

const hostedZoneStack = new HostZoneStack(app, "HostedZoneStack", {
    domain: "somedomain.com"
});

new ApplicationStack(app, "StaticSiteStack", {
    hostedZone: hostedZoneStack.hostedZone, // imported zone passed in the stack props
    uiDomain: "ui.somedomain.com",
    env: {
        region: "us-east-1"
    }
});

@bind-almir
Copy link

I had the same issue and came up with this workaround:

this.hostedZoneProvider = new HostedZoneProvider(this, {
  domainName: props.domainName,
  privateZone: props.privateZone
});

const zone: HostedZoneAttributes = this.hostedZoneProvider.findHostedZone();

this.hostedZone = HostedZone.fromHostedZoneAttributes(this, 'HostedZone', {
  hostedZoneId: zone.hostedZoneId,
  zoneName: zone.zoneName
});

I am not sure was this right thing to do but it works for me. Is there a better solution?

@rpanfili
Copy link
Contributor

I found, as a workaround, that if the hosted zone reference is still unresolved and contains the dummy values (example.com) you can simply return and wait for the next AppStack synth iteration when cdk will use the HostedZoneContextProviderPlugin to get real values from your account.

Take a look here:
https://github.com/awslabs/aws-cdk/blob/23a143ea34a1654af24bf8c58f73da26136d039e/packages/aws-cdk/lib/api/cxapp/stacks.ts#L205-L207

So a working (boilerplate) solution is:

  const domainName = "foobar.com";

  const zone = new HostedZoneProvider(this, {
    domainName
  }).findAndImport(this, "HostedZone");
  if (zone.zoneName == 'example.com') {
    return;
  }
  const certificate = new DnsValidatedCertificate(this, 'ApiSSLCertificate', {
    domainName,
    hostedZone: zone
  });

Warning: Obviously the given domainName hosted zone should exists in your account otherwise you will get an error like this

Found zones: [] for dns:foobar.com, privateZone:undefined, vpcId:undefined, but wanted exactly 1 zone

@BlueYetii
Copy link

Nice workaround @rpanfili, much easier solution to the issue.

rix0rrr added a commit that referenced this issue Jun 21, 2019
Validation errors are currently turned into exceptions. This causes
issues where missing context values lead to validation errors (such as
in the case of `DnsValidatedCertificate`): the error would have been
solved by retrieving the context value, but because an exception is
thrown the CLI stops immediately and doesn't re-execute.

I feel it also makes sense to treat these errors as construct errors,
as opposed to exceptions which I feel should be more "you're misusing
the API" kind of errors, rather than "something is wrong somewhere"
kind of errors.

The change I made is the smallest change to achieve the desired effect.
If we agree that we want this change, it would probably be better to
do it differently, just have `.validate()` implementors call
`this.node.addError()` immediately.

Fixes #2076.
rix0rrr added a commit that referenced this issue Jun 21, 2019
Validation errors are currently turned into exceptions. This causes
issues where missing context values lead to validation errors (such as
in the case of `DnsValidatedCertificate`): the error would have been
solved by retrieving the context value, but because an exception is
thrown the CLI stops immediately and doesn't re-execute.

I feel it also makes sense to treat these errors as construct errors,
as opposed to exceptions which I feel should be more "you're misusing
the API" kind of errors, rather than "something is wrong somewhere"
kind of errors.

The change I made is the smallest change to achieve the desired effect.
If we agree that we want this change, it would probably be better to
do it differently, just have `.validate()` implementors call
`this.node.addError()` immediately.

Fixes #2076.
rix0rrr added a commit that referenced this issue Jun 21, 2019
Be sure that the dummy hosted zone props that are returned before
the actual lookup is done have the right domain name already. This is
necessary to make DnsValidateCertificate's validation check succeed.

Fixes #2076.
rix0rrr added a commit that referenced this issue Jun 21, 2019
Be sure that the dummy hosted zone props that are returned before
the actual lookup is done have the right domain name already. This is
necessary to make DnsValidateCertificate's validation check succeed.

Fixes #2076.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment