From aa4839e7482f5f67dfff670ade3d5c84913be20a Mon Sep 17 00:00:00 2001 From: Michael Leditschke Date: Sat, 25 Dec 2021 13:16:10 +1100 Subject: [PATCH 1/2] refactor: remove implicit Enabled attribute - remove the behaviour were an `Enabled` attribute was added by default to composite objects - remove use of the `InihibitEnabled` attribute to suppress the default behaviour - add explicit `Enabled` attributes where they are currently expected by code - extend the `isPresent` macro to throw an error if it doesn't find an `Enabled` attribute isPresent - restrict the specification of attributes for composite object to use the full object based notation, removing the support for string based attributes. --- README.md | 2 +- engine/base.ftl | 143 +++++++----------- engine/configuration/component.ftl | 5 + engine/configuration/layer.ftl | 5 - engine/configuration/module.ftl | 5 +- engine/configuration/task.ftl | 5 +- engine/extension.ftl | 1 - engine/inputdata/layer.ftl | 1 - engine/occurrence.ftl | 2 +- .../shared/attributesets/containertask/id.ftl | 5 +- providers/shared/components/apigateway/id.ftl | 15 ++ providers/shared/components/component.ftl | 41 ++++- providers/shared/components/lambda/id.ftl | 10 ++ providers/shared/components/lb/id.ftl | 10 ++ providers/shared/components/s3/id.ftl | 10 ++ providers/shared/components/sqs/id.ftl | 5 + providers/shared/layers/Segment/id.ftl | 5 + 17 files changed, 163 insertions(+), 107 deletions(-) diff --git a/README.md b/README.md index e043a4b3a..02afbf46a 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ To run the test suite locally install the hamlet cli and use the provider testin pip install hamlet # run the tests -hamlet -p shared -p sharedtest -f default deploy run-deployments +hamlet -i mock -p shared -p sharedtest -f default deploy run-deployments -o /tmp/results_dir ``` This will run all of the tests and provide you the results. We also run this on all Pull requests made to the repository diff --git a/engine/base.ftl b/engine/base.ftl index aeabcabef..f936a17b0 100644 --- a/engine/base.ftl +++ b/engine/base.ftl @@ -1258,11 +1258,6 @@ are added. [#if normalisedAttributes?has_content] [#list normalisedAttributes as attribute] - [#-- Ignore any inhibit enabled marker --] - [#if ! attribute?is_hash] - [#continue] - [/#if] - [#local populateMissingChildren = attribute.PopulateMissingChildren ] [#-- TODO(mfl) Should all name alternatives be processed? Doing so would represent a change in behaviour --] @@ -1669,96 +1664,56 @@ are added. [#function normaliseCompositeConfiguration attributes ] [#-- Normalise attributes --] [#local normalisedAttributes = [] ] - [#local inhibitEnabled = false] - [#local explicitEnabled = false] + [#if attributes?has_content] [#list asFlattenedArray(attributes) as attribute] [#local names = [] ] - [#if attribute?is_string] - [#if attribute == "InhibitEnabled" ] - [#local inhibitEnabled = true ] - [#continue] - [/#if] - [#-- Defaults if only the attribute name is provided --] - [#local normalisedAttribute = - { - "Names" : [attribute], - "Types" : [ANY_TYPE], - "Mandatory" : false, - "DefaultBehaviour" : "ignore", - "DefaultProvided" : false, - "Values" : [], - "Children" : [], - "SubObjects" : false, - "PopulateMissingChildren" : true, - "AttributeSet" : "", - "Component" : "", - "Description" : "" - } - ] + [#if ! attribute?is_hash] + [@warn + message="Only object based attribute configurations are supported. Attribute will be ignored." + context=attributes + detail=attribute + /] + [#continue] [/#if] - [#if attribute?is_hash ] - [#local names = attribute.Names!"Hamlet:Missing" ] - [#if (names?is_string) && (names == "Hamlet:Missing") ] - [@fatal - message="Attribute must have a \"Names\" attribute" - context=attribute - /] - [/#if] - [#local normalisedAttribute = - { - "Names" : asArray(names), - "Types" : asArray(attribute.Types!attribute.Type!ANY_TYPE), - "Mandatory" : attribute.Mandatory!false, - "DefaultBehaviour" : attribute.DefaultBehaviour!"ignore", - "DefaultProvided" : attribute.Default??, - "Values" : asArray(attribute.Values![]), - "Children" : normaliseCompositeConfiguration( - asArray(attribute.Children![]) - ), - "SubObjects" : attribute.SubObjects!attribute.Subobjects!false, - "PopulateMissingChildren" : attribute.PopulateMissingChildren!true, - "AttributeSet" : attribute.AttributeSet!"", - "Component" : attribute.Component!"", - "Description" : attribute.Description!"" - } + - attributeIfTrue( - "Default", - attribute.Default??, - attribute.Default!"" - ) - ] + [#local names = attribute.Names!"Hamlet:Missing" ] + [#if (names?is_string) && (names == "Hamlet:Missing") ] + [@fatal + message="Attribute must have a \"Names\" attribute" + context=attribute + /] [/#if] + [#local normalisedAttribute = + { + "Names" : asArray(names), + "Types" : asArray(attribute.Types!attribute.Type!ANY_TYPE), + "Mandatory" : attribute.Mandatory!false, + "DefaultBehaviour" : attribute.DefaultBehaviour!"ignore", + "DefaultProvided" : attribute.Default??, + "Values" : asArray(attribute.Values![]), + "Children" : normaliseCompositeConfiguration( + asArray(attribute.Children![]) + ), + "SubObjects" : attribute.SubObjects!attribute.Subobjects!false, + "PopulateMissingChildren" : attribute.PopulateMissingChildren!true, + "AttributeSet" : attribute.AttributeSet!"", + "Component" : attribute.Component!"", + "Description" : attribute.Description!"" + } + + attributeIfTrue( + "Default", + attribute.Default??, + attribute.Default!"" + ) + ] + [#local normalisedAttributes += [normalisedAttribute] ] - [#local explicitEnabled = explicitEnabled || normalisedAttribute.Names?seq_contains("Enabled") ] [/#list] - [#if (!explicitEnabled) && (!inhibitEnabled) ] - [#-- Put "Enabled" first to ensure it is processed in case a name of "*" is used --] - [#local normalisedAttributes = - [ - { - "Names" : ["Enabled"], - "Types" : [BOOLEAN_TYPE], - "Mandatory" : false, - "DefaultBehaviour" : "ignore", - "DefaultProvided" : true, - "Default" : true, - "Values" : [], - "Children" : [], - "SubObjects" : false, - "PopulateMissingChildren" : true, - "AttributeSet" : "", - "Component" : "", - "Description" : "" - } - ] + - normalisedAttributes ] - [/#if] [/#if] - [#return normalisedAttributes + arrayIfTrue("InhibitEnabled", inhibitEnabled) ] + [#return normalisedAttributes ] [/#function] [#macro validateCompositeObject attributes=[] objects=[] ] @@ -1909,9 +1864,23 @@ are added. [#return combineEntities(frameworkObjectAttributes, result, ADD_COMBINE_BEHAVIOUR)] [/#function] -[#-- Check if a configuration item with children is present --] +[#-- Check if a configuration item with children is present --] [#function isPresent configuration={} ] - [#return (configuration.Configured!false) && (configuration.Enabled!false) ] + [#if ! configuration.Configured?? ] + [@fatal + message="internal error - isPresent() should only be called where child population has been configured" + context=.caller_template_name + /] + [#return false] + [/#if] + [#if ! configuration.Enabled?? ] + [@fatal + message="internal error - isPresent() should only be called where an Enabled attribute has been configured" + context=.caller_template_name + /] + [#return false] + [/#if] + [#return (configuration.Configured) && (configuration.Enabled) ] [/#function] [#function getObjectLineage collection end ] @@ -2856,7 +2825,7 @@ expression format provided by the long form. --] [#function getQualifierChildren filterAttributes=[] ] - [#local filterChildren = ["InhibitEnabled"] ] + [#local filterChildren = [] ] [#list filterAttributes as filterAttribute] [#local filterChildren += [ diff --git a/engine/configuration/component.ftl b/engine/configuration/component.ftl index 70b1b176a..9b2b7619d 100644 --- a/engine/configuration/component.ftl +++ b/engine/configuration/component.ftl @@ -189,6 +189,11 @@ [#macro addComponentBase type includeTypeAttr=true ] [#local attributes = [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Export", "Types" : BOOLEAN_TYPE, diff --git a/engine/configuration/layer.ftl b/engine/configuration/layer.ftl index 829d0c3fd..c066bb97c 100644 --- a/engine/configuration/layer.ftl +++ b/engine/configuration/layer.ftl @@ -13,11 +13,6 @@ [#-- contains common or reference attribute values for the layer --] [#macro addLayer type referenceLookupType properties attributes inputFilterAttributes=[] ] - [#local attributes = attributes?seq_contains("InhibitEnabled")?then( - attributes, - combineEntities(asArray(attributes), [ "InhibitEnabled"], APPEND_COMBINE_BEHAVIOUR) - )] - [#local configuration = { "Type" : type, diff --git a/engine/configuration/module.ftl b/engine/configuration/module.ftl index 1860ec951..f78f6c7dc 100644 --- a/engine/configuration/module.ftl +++ b/engine/configuration/module.ftl @@ -23,10 +23,7 @@ configuration={ "Provider" : provider } - attributes=properties?seq_contains("InhibitEnabled")?then( - properties, - combineEntities(asArray(properties), [ "InhibitEnabled"], APPEND_COMBINE_BEHAVIOUR) - ) + attributes=properties /] [/#macro] diff --git a/engine/configuration/task.ftl b/engine/configuration/task.ftl index ebaa59da4..28ad38600 100644 --- a/engine/configuration/task.ftl +++ b/engine/configuration/task.ftl @@ -16,10 +16,7 @@ scopeId=TASK_CONFIGURATION_SCOPE id=type properties=properties - attributes=attributes?seq_contains("InhibitEnabled")?then( - attributes, - combineEntities(asArray(attributes), [ "InhibitEnabled"], APPEND_COMBINE_BEHAVIOUR) - ) + attributes=attributes /] [/#macro] diff --git a/engine/extension.ftl b/engine/extension.ftl index 35cfbbe77..0c9feea1b 100644 --- a/engine/extension.ftl +++ b/engine/extension.ftl @@ -67,7 +67,6 @@ [#macro addExtension id description supportedTypes aliases=[] entrances=["deployment"] scopes=[ SETUP_EXTENSION_SCOPE ] provider=SHARED_PROVIDER ] [#local extensionConfiguration = [ - "InhibitEnabled", { "Names" : "Id", "Type" : STRING_TYPE diff --git a/engine/inputdata/layer.ftl b/engine/inputdata/layer.ftl index cce257e68..971d813b9 100644 --- a/engine/inputdata/layer.ftl +++ b/engine/inputdata/layer.ftl @@ -83,7 +83,6 @@ filter, blueprint, [ - "InhibitEnabled", { "Names" : "Id", "Types" : STRING_TYPE diff --git a/engine/occurrence.ftl b/engine/occurrence.ftl index f830d23c5..298227c5a 100644 --- a/engine/occurrence.ftl +++ b/engine/occurrence.ftl @@ -765,7 +765,7 @@ already prefixed attribute. [/#if] [/#list] - [#return asFlattenedSettings(getCompositeObject(["InhibitEnabled", { "Names" : "*" }], contexts)) ] + [#return asFlattenedSettings(getCompositeObject([{ "Names" : "*" }], contexts)) ] [/#function] [#function internalGetOccurrenceSetting occurrence names emptyIfNotProvided=false] diff --git a/providers/shared/attributesets/containertask/id.ftl b/providers/shared/attributesets/containertask/id.ftl index 4c12e8b9e..1429869d1 100644 --- a/providers/shared/attributesets/containertask/id.ftl +++ b/providers/shared/attributesets/containertask/id.ftl @@ -82,7 +82,10 @@ "Names" : "Ports", "SubObjects" : true, "Children" : [ - "Container", + { + "Names": "Container", + "Types": STRING_TYPE + }, { "Names" : "DynamicHostPort", "Types" : BOOLEAN_TYPE, diff --git a/providers/shared/components/apigateway/id.ftl b/providers/shared/components/apigateway/id.ftl index c06d433c2..1698ff316 100644 --- a/providers/shared/components/apigateway/id.ftl +++ b/providers/shared/components/apigateway/id.ftl @@ -110,6 +110,11 @@ object. { "Names" : "CloudFront", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "AssumeSNI", "Types" : BOOLEAN_TYPE, @@ -202,6 +207,11 @@ object. "Names" : "Publish", "Description" : "Deprecated - Please switch to the publishers configuration", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "DnsNamePrefix", "Types" : STRING_TYPE, @@ -243,6 +253,11 @@ object. { "Names" : "Mapping", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + } { "Names" : "IncludeStage", "Types" : BOOLEAN_TYPE, diff --git a/providers/shared/components/component.ftl b/providers/shared/components/component.ftl index ea109f5df..ad1823c79 100644 --- a/providers/shared/components/component.ftl +++ b/providers/shared/components/component.ftl @@ -222,6 +222,11 @@ { "Names" : "Stepped", "Children" : [ + { + "Names" : "Enabled", + "Type" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "MetricAggregation", "Description" : "The method used to agregate the cloudwatch metric", @@ -270,6 +275,11 @@ { "Names" : "Tracked", "Children" : [ + { + "Names" : "Enabled", + "Type" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "TargetValue", "Types" : NUMBER_TYPE @@ -289,6 +299,11 @@ { "Names" : "Scheduled", "Children" : [ + { + "Names" : "Enabled", + "Type" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "ProcessorProfile", "Types" : STRING_TYPE, @@ -369,6 +384,11 @@ ] [#assign wafChildConfiguration = [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Version", "Description" : "Version of WAF to use", @@ -513,6 +533,7 @@ } ]] +[#-- SHOULDN'T have Enabled attribute --] [#assign domainChildConfiguration = [ { "Names" : "Name", @@ -538,8 +559,7 @@ "Names" : "Role", "Types" : STRING_TYPE, "Values" : [DOMAIN_ROLE_PRIMARY, DOMAIN_ROLE_SECONDARY] - }, - "InhibitEnabled" + } ]] [#assign domainNameChildConfiguration = [ @@ -616,6 +636,13 @@ ]] [#assign certificateChildConfiguration = + [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + } + ] + domainNameChildConfiguration + hostNameChildConfiguration + [ @@ -1336,6 +1363,11 @@ "Names" : "Schedules", "SubObjects" : true, "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Expression", "Types" : STRING_TYPE, @@ -1416,6 +1448,11 @@ [#assign tracingChildConfiguration = [ + { + "Names" : "Enabled", + "Type" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Mode", "Types" : STRING_TYPE, diff --git a/providers/shared/components/lambda/id.ftl b/providers/shared/components/lambda/id.ftl index 3737637b2..8fa0fd19a 100644 --- a/providers/shared/components/lambda/id.ftl +++ b/providers/shared/components/lambda/id.ftl @@ -104,6 +104,11 @@ "Names" : "Schedules", "SubObjects" : true, "Children" : [ + { + "Names" : "Enabled", + "Type" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Expression", "Types" : STRING_TYPE, @@ -173,6 +178,11 @@ { "Names" : "FixedCodeVersion", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "NewVersionOnDeploy", "Types" : BOOLEAN_TYPE, diff --git a/providers/shared/components/lb/id.ftl b/providers/shared/components/lb/id.ftl index 5ed27d724..72291577d 100644 --- a/providers/shared/components/lb/id.ftl +++ b/providers/shared/components/lb/id.ftl @@ -240,6 +240,11 @@ "Names" : "Redirect", "Description" : "When the rule matches send a redirect response to the client", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Protocol", "Types" : STRING_TYPE, @@ -277,6 +282,11 @@ "Names" : "Fixed", "Description" : "When the rule is matched send a fixed http response to the client", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Message", "Types" : STRING_TYPE, diff --git a/providers/shared/components/s3/id.ftl b/providers/shared/components/s3/id.ftl index 9bbbf4c4c..75cd30441 100644 --- a/providers/shared/components/s3/id.ftl +++ b/providers/shared/components/s3/id.ftl @@ -24,6 +24,11 @@ { "Names" : "Lifecycle", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Expiration", "Types" : [STRING_TYPE, NUMBER_TYPE], @@ -51,6 +56,11 @@ { "Names" : "Website", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names": "Index", "Types" : STRING_TYPE, diff --git a/providers/shared/components/sqs/id.ftl b/providers/shared/components/sqs/id.ftl index 90faa8b40..34b0ff5bf 100644 --- a/providers/shared/components/sqs/id.ftl +++ b/providers/shared/components/sqs/id.ftl @@ -35,6 +35,11 @@ "Names" : "DeadLetterQueue", "Description" : "Enables a dead letter queue for messages which reach a specified retry count", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "MaxReceives", "Description" : "The maximum number of times a single message can be put back on the queue", diff --git a/providers/shared/layers/Segment/id.ftl b/providers/shared/layers/Segment/id.ftl index 51ec7f8f9..8bbd9c81c 100644 --- a/providers/shared/layers/Segment/id.ftl +++ b/providers/shared/layers/Segment/id.ftl @@ -154,6 +154,11 @@ { "Names" : "CIDR", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Address", "Types" : STRING_TYPE, From 2bf12c8c6b50b39ae116d25dda35ba7bdecd9270 Mon Sep 17 00:00:00 2001 From: Michael Leditschke Date: Sat, 25 Dec 2021 13:57:09 +1100 Subject: [PATCH 2/2] refactor: Extra Enabled attributes for Azure Add `Enabled` attributes required to run the Azure test suite. Also correct the README in regards to running the unit tests. --- README.md | 2 +- providers/shared/references/Port/id.ftl | 5 +++++ providers/shared/references/TestCase/id.ftl | 10 ++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 02afbf46a..b470c8400 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ To run the test suite locally install the hamlet cli and use the provider testin pip install hamlet # run the tests -hamlet -i mock -p shared -p sharedtest -f default deploy run-deployments -o /tmp/results_dir +hamlet -i mock -p shared -p sharedtest -f default deploy test-deployments -o /tmp/results_dir ``` This will run all of the tests and provide you the results. We also run this on all Pull requests made to the repository diff --git a/providers/shared/references/Port/id.ftl b/providers/shared/references/Port/id.ftl index 0b93937da..3109617b2 100644 --- a/providers/shared/references/Port/id.ftl +++ b/providers/shared/references/Port/id.ftl @@ -84,6 +84,11 @@ { "Names" : "PortRange", "Children" : [ + { + "Names" : "Enabled", + "Types" : BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "From", "Types" : NUMBER_TYPE diff --git a/providers/shared/references/TestCase/id.ftl b/providers/shared/references/TestCase/id.ftl index f186d0591..847518dfb 100644 --- a/providers/shared/references/TestCase/id.ftl +++ b/providers/shared/references/TestCase/id.ftl @@ -109,6 +109,11 @@ "Names" : "cfn-lint", "Description" : "Run cfn-lint on the file - https://github.com/aws-cloudformation/cfn-lint", "Children" : [ + { + "Names": "Enabled", + "Type": BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "IgnoreChecks", "Description" : "A list of checks to ignore in cfn-lint", @@ -121,6 +126,11 @@ "Names" : "checkov", "Description" : "Run checkov on the file - https://github.com/bridgecrewio/checkov", "Children" : [ + { + "Names": "Enabled", + "Type": BOOLEAN_TYPE, + "Default" : true + }, { "Names" : "Framework", "Description" : "The framework of the file to run testing against",