Domain manager output failing serverless compose

alice-byb commented 2 years ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Bug Report

Error Description

When running serverless info, a string of hyphens (-) are printed to the console. these hyphens are not yaml-compatible, which breaks tools expecting yaml-compatible content from serverless info.

This is an issue for use with serverless compose which uses the output from serverless info in YAML form to retrieve outputs for use in other services. (see serverless compose code here)

I'm not sure whether the logs printed during after:info:info are intended to be strict YAML as mentioned in this PR discussion but it seems trivial to fix this issue for serverless-domain-manager

I believe this to be a problem for other serverless plugins (this was the case for a plugin I am maintaining also) as well so it may be best to open the discussion for serverless compose but in the short-term this should be resolvable. I have mentioned more about the issue/fix in the section below. Please let me know if I should open a PR to resolve this.

Command Run

serverless info

Console Output

Stack Outputs:
  DomainName: dev-test-api.xxx.com
  HostedZoneId: xxxx
  BackendLambdaFunctionQualifiedArn: arn:aws:lambda:us-west-2:xxxx:function:backend-dev-backend:1
  DistributionDomainName: xxxx.cloudfront.net
  ServiceEndpoint: https://xxxx.execute-api.us-west-2.amazonaws.com/dev
  ServerlessDeploymentBucketName: backend-dev-serverlessdeploymentbucket-xxxx
Serverless Domain Manager:
  Domain Name: dev-test-api.xxxx.com
  Target Domain: xxxx.cloudfront.net
  Hosted Zone Id: xxxx
  ----------------------------------------

Domain Manager Configuration

I have created an example repo here with setup instructions to replicate the issue/workflow. Below is the config of the service in the example repo

custom:
  customDomain:
    autoDomain: true
    basePath: ''
    certificateName: '*.${self:custom.domain}'
    createRoute53Record: true
    domainName: ${self:custom.${self:custom.stage}.api_domain}
    stage: ${self:provider.stage}

Versions

Domain Manager version(s): v6.0.3
Node/npm version: node 14 & 16
Serverless Version: >=3.15.0
Lambda Code: JavaScript

Possible Solution

The potential fix to this issue is to prepend the row of hyphens with a yaml comment character # fix would allow the output to parsed. I have added a code example of potential fix to the code in question below:

    public static printDomainSummary(domains: DomainConfig[]): void {
        const summaryList = [];
        domains.forEach((domain) => {
            if (domain.domainInfo) {
                summaryList.push(`Domain Name: ${domain.givenDomainName}`);
                summaryList.push(`Target Domain: ${domain.domainInfo.domainName}`);
                summaryList.push(`Hosted Zone Id: ${domain.domainInfo.hostedZoneId}`);
               -summaryList.push(`----------------------------------------`);
               +summaryList.push(`# ----------------------------------------`);

Additional context/Screenshots

As mentioned above, I have created a sample repo to reproduce the problem and have a screenshot of the output:

mnapoli commented 2 years ago

To be honest I don't think this issue should be solved here. We're working on solving this in Compose directly.

However, since we are here, as a general recommendation I would advise not to print the ----------- line in the Serverless Framework output. I guess the goal is to separate custom domains? Maybe the output of the plugin could be reworked to something like this:

Serverless Domain Manager:
  domain1.example.com:
    Target Domain: xxxx.cloudfront.net
    Hosted Zone Id: ...
  domain2.example.com:
    Target Domain: yyyy.cloudfront.net
    Hosted Zone Id: ...

Cesaraugp commented 2 years ago

Any updates on this? @mnapoli

rddimon commented 2 years ago

Hi there

Thank you for your issue!

@bailey-byb @Cesaraugp feel free to create a PR with changes. We will review and merge it.

mnapoli commented 2 years ago

@Cesaraugp yes, the issue is solved in Compose, so at least there's no bug anymore.

I still think it could be worth improving this part in the plugin: https://github.com/amplify-education/serverless-domain-manager/issues/500#issuecomment-1110709980

amplify-education / serverless-domain-manager

Domain manager output failing serverless compose #500

Community Note

Bug Report