awsdocs / aws-cloudformation-user-guide

The open source version of the AWS CloudFormation User Guide
Other
766 stars 1.3k forks source link

`AWS::RDS::DBCluster` `MasterUsername` and `MasterUserPassword` should be listed as `Required: Yes` #953

Closed LJArendse closed 3 years ago

LJArendse commented 3 years ago

The current Cloudformation User Guide for AWS::RDS::DBCluster lists both the MasterUsername and MasterUserPassword properties as Required: No (see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html).

Issue Labels: Resource Specification and/or question

Whilst experimenting and deploying my own RDS DB cluster with the following properties (note this is just a subset of my properties):

RDSCluster:
    Type: "AWS::RDS::DBCluster"
    Properties:
      AvailabilityZones:
        - af-south-1a
      Engine: aurora-postgresql
      EngineMode: provisioned
      EngineVersion: 12.4
      DBClusterParameterGroupName: default.aurora-postgresql12
      .
      .
      .

Please note that I am not listing the MasterUsername and MasterUserPassword properties. Additionally, I did not specify the SourceDBInstanceIdentifier or SnapshotIdentifier.

Cloudformation throws the following errors when either the MasterUsername and/or MasterUserPassword properties are omitted. See error message screenshots below: error_omit_MasterUsername error_omit_MasterUserPassword Therefore, to me at least, it looks like the MasterUsername and MasterUserPassword properties should be listed as Required: Yes.

From what I know, the Required, Type, and Update requires fields are auto-generated from the AWS CloudFormation resource specification. I am logging the issue here, as I am not sure where to log a ticket for the fix in the resource specification: i.e. af-south-1 CloudFormationResourceSpecification.json:

        "MasterUserName": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticsearch-domain-masteruseroptions.html#cfn-elasticsearch-domain-masteruseroptions-masterusername",
          "PrimitiveType": "String",
          "Required": true,
          "UpdateType": "Mutable"
        },
        "MasterUserPassword": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticsearch-domain-masteruseroptions.html#cfn-elasticsearch-domain-masteruseroptions-masteruserpassword",
          "PrimitiveType": "String",
          "Required": true,
          "UpdateType": "Mutable"
        }
      }

If anyone has any additional insight or guidance please feel free to comment. I might be missing something obvious, so please comment.

rachfop commented 3 years ago

You are correct that the Required, Type, and Update required fields are auto-generated. I would suggest changing the Required field from No to Conditional. Per the note:

If you specify the SourceDBInstanceIdentifier or SnapshotIdentifier property, don't specify this property. The value is inherited from the source DB instance or snapshot.

randyurbano commented 3 years ago

I updated the documentation for MasterUserName and MasterUserPassword. They now have Required: Conditional.

LJArendse commented 3 years ago

Hi @randyurbano and @rachfop, thanks for the clarification and help. The Required: Conditional makes the most sense šŸ„‡

I am happy to close this issue, if you are?

randyurbano commented 3 years ago

Yes, I'll close it. Thanks again for your feedback.