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

Added SGR validation to cfn validate #1070

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ammokhov
Copy link
Contributor

Issue #, if available:

Description of changes:

This is a draft PR and subject to discussion.

This change enhances cfn validate command by invoking https://github.com/aws-cloudformation/resource-schema-guard-rail/ for schema compliance evaluation.

It invokes both stateful and stateless assessment.

Stateless

This evaluation runs on the current version of the resource schema

Stateful

This attempts to retrieve original schema from CloudFormation registry by calling describe_type.

If for whatever reason API call has not succeeded it will be recorded in logs but will not fail the command.

Some CX Considerations

API Call to Retrieve schema from registry is a default behavior for backward compatibility compliance assessment (stateful). Perhaps, it is useful to provide an argument to the user to specify static file of the original schema (without making an api call)

Example Output:

•100%     cfn validate
Type Exists in CloudFormation Registry. Evaluating Resource Schema Backward Compatibility Compliance
────────────────────────────────────────────────────────────────────────────────────────────── [GENERATED DIFF BETWEEN SCHEMAS] ──────────────────────────────────────────────────────────────────────────────────────────────
{
    'description': {
        'changed': [
            {
                'property': '',
                'old_value': 'The AWS::KMS::Alias resource specifies a display name for a customer master key (CMK) in AWS Key Management Service (AWS KMS). You can use an alias to identify a CMK in cryptographic operations. ',
                'new_value': 'The AWS::KMS::Alias resource specifies a display name for an AWS KMS key in AWS Key Management Service (AWS KMS). You can use an alias to identify an AWS KMS key in cryptographic operations.'
            },
            {
                'property': '/properties/AliasName',
                'old_value': 'Specifies the alias name. This value must begin with alias/ followed by a name, such as alias/ExampleAlias. The alias name cannot begin with alias/aws/. The alias/aws/ prefix is reserved for AWS managed CMKs.',
                'new_value': 'Specifies the alias name. This value must begin with alias/ followed by a name, such as alias/ExampleAlias. The alias name cannot begin with alias/aws/. The alias/aws/ prefix is reserved for AWS managed keys.'
            },
            {
                'property': '/properties/TargetKeyId',
                'old_value': 'Identifies the CMK to which the alias refers. Specify the key ID or the Amazon Resource Name (ARN) of the CMK. You cannot specify another alias. For help finding the key ID and ARN, see Finding the Key ID and ARN in the AWS Key Management Service Developer Guide.',
                'new_value': 'Identifies the AWS KMS key to which the alias refers. Specify the key ID or the Amazon Resource Name (ARN) of the AWS KMS key. You cannot specify another alias. For help finding the key ID and ARN, see Finding the Key ID and ARN in the AWS Key Management Service Developer Guide.'
            }
        ]
    }
}
                               Schema Compliance Report
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━┳━━━━━━┳━━━━━━━━━┓
┃                                     Rule Name ┃ Check Id ┃ Message ┃ Path ┃  Status ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━╇━━━━━━╇━━━━━━━━━┩
│                 ensure_minimum_not_contracted │ -        │ -       │ -    │ skipped │
│                ensure_minitems_not_contracted │ -        │ -       │ -    │ skipped │
│                       ensure_enum_not_changed │ -        │ -       │ -    │ skipped │
│               ensure_maxlength_not_contracted │ -        │ -       │ -    │ skipped │
│      ensure_old_property_not_turned_writeonly │ -        │ -       │ -    │ skipped │
│ ensure_old_property_not_removed_from_readonly │ -        │ -       │ -    │ skipped │
│               ensure_minlength_not_contracted │ -        │ -       │ -    │ skipped │
│                ensure_maxitems_not_contracted │ -        │ -       │ -    │ skipped │
│               ensure_old_property_not_removed │ -        │ -       │ -    │ skipped │
│            ensure_no_more_required_properties │ -        │ -       │ -    │ skipped │
│                 ensure_maximum_not_contracted │ -        │ -       │ -    │ skipped │
│      ensure_old_property_not_turned_immutable │ -        │ -       │ -    │ skipped │
│         ensure_primary_identifier_not_changed │ -        │ -       │ -    │ skipped │
│              ensure_property_type_not_changed │ -        │ -       │ -    │ skipped │
│    ensure_property_string_pattern_not_changed │ -        │ -       │ -    │ skipped │
└───────────────────────────────────────────────┴──────────┴─────────┴──────┴─────────┘
Evaluating Resource Schema Compliance
                                                                       Schema Compliance Report
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━┓
┃                                                  Rule Name ┃ Check Id ┃ Message                                                  ┃ Path                  ┃  Status ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━┩
│                      ensure_arn_properties_contain_pattern │ -        │ -                                                        │ -                     │ skipped │
│                          ensure_arn_properties_type_string │ -        │ -                                                        │ -                     │ skipped │
│                              ensure_array_doesnt_use_anyof │ -        │ -                                                        │ -                     │ skipped │
│          ensure_create_and_read_only_intersection_is_empty │ -        │ -                                                        │ -                     │ skipped │
│           ensure_write_and_read_only_intersection_is_empty │ -        │ -                                                        │ -                     │ skipped │
│                         ensure_default_replacementStrategy │ -        │ -                                                        │ -                     │ skipped │
│                             ensure_property_tags_exists_v2 │ -        │ -                                                        │ -                     │ skipped │
│                             ensure_property_tags_exists_v1 │ -        │ -                                                        │ -                     │ skipped │
│                 ensure_properties_do_not_support_multitype │ -        │ -                                                        │ -                     │  passed │
│   ensure_resource_list_handler_exists_and_have_permissions │ -        │ -                                                        │ -                     │  passed │
│ ensure_resource_delete_handler_exists_and_have_permissions │ -        │ -                                                        │ -                     │  passed │
│   ensure_resource_read_handler_exists_and_have_permissions │ -        │ -                                                        │ -                     │  passed │
│ ensure_resource_create_handler_exists_and_have_permissions │ -        │ -                                                        │ -                     │  passed │
│ ensure_resource_update_handler_exists_and_have_permissions │ -        │ -                                                        │ -                     │  passed │
│                          ensure_description_is_descriptive │ -        │ -                                                        │ -                     │  passed │
│                                ensure_sourceUrl_uses_https │ -        │ -                                                        │ -                     │  passed │
│                                   verify_property_notation │ -        │ -                                                        │ -                     │  passed │
│             ensure_primary_identifier_exists_and_not_empty │ -        │ -                                                        │ -                     │  passed │
│                 ensure_taggable_and_tagging_do_not_coexist │ -        │ -                                                        │ -                     │  passed │
│                                  check_if_taggable_is_used │ TAG001   │ `taggable` is deprecated, please used `tagging` property │ -                     │ warning │
│           ensure_primary_identifier_is_read_or_create_only │ PID003   │ primaryIdentifier MUST be either readOnly or createOnly  │ /createOnlyProperties │  failed │
│                                ensure_tagging_is_specified │ TAG002   │ `tagging` MUST be specified                              │ -                     │  failed │
└────────────────────────────────────────────────────────────┴──────────┴──────────────────────────────────────────────────────────┴───────────────────────┴─────────┘
Explicitly specify value for tagging
Resource schema is valid

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@prerna-p
Copy link
Member

prerna-p commented May 21, 2024

Can we remove the Resource schema is valid message if there are failed checks? Instead output something like Issues were found in resource schema

src/rpdk/core/project.py Show resolved Hide resolved
@@ -275,7 +275,7 @@ def validate_and_load_resource_settings(self, raw_settings):
self.entrypoint = raw_settings["entrypoint"]
self.test_entrypoint = raw_settings["testEntrypoint"]
self.executable_entrypoint = raw_settings.get("executableEntrypoint")
self._plugin = load_plugin(raw_settings["language"])
# self._plugin = load_plugin(raw_settings["language"])
Copy link
Contributor

Choose a reason for hiding this comment

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

? Whats the impact to this or why do we need to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad, this is just for my local setup. I included it by mistake

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.

None yet

3 participants