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

Jump to bottom

addresses #1556 by making 'condition' optional for automations #378

Merged
merged 7 commits into from Aug 8, 2019
Merged

addresses #1556 by making 'condition' optional for automations #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
659cdfa
edit: update test setup to use map instead of anonymous struct
aaslamin Aug 6, 2019
fb8da3b
addresses #1556 by making the 'condition' attribute optional if and o…
aaslamin Aug 6, 2019
29dfd6e
fix: broken tests as a result of new custom validation logic
aaslamin Aug 6, 2019
90479f3
new: add test coverage to verify behaviour for new custom validation …
aaslamin Aug 6, 2019
55a2cc0
fix: add Webhook to spelling whitelist
aaslamin Aug 6, 2019
79507d9
edit: make condition optional for all types of automations
aaslamin Aug 7, 2019
a69858b
edit: update comment copy
aaslamin Aug 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .spelling
View file
Open in desktop
Expand Up @@ -242,6 +242,7 @@ unlogged
userland
vCPUs
vince
Webhook
whitelist
whitelisted
whitelisting
Expand Down
50 changes: 28 additions & 22 deletions automation.go
View file
Open in desktop
Expand Up @@ -112,8 +112,11 @@ type Automation struct {
// List of tags attached to an entity.
AssociatedTags []string `json:"associatedTags" msgpack:"associatedTags" bson:"associatedtags" mapstructure:"associatedTags,omitempty"`

// Condition contains the code that will be executed to decide if any action should
// be taken.
// Condition contains the code that will be executed to decide if any action(s)
// should be executed. Providing a condition for an automation with a
// "Webhook" trigger type will have no impact as the condition will not be
// evaluated. If no condition is defined, then the automation action(s) will be executed;
// this behaves akin to a condition that always succeeds.
Condition string `json:"condition" msgpack:"condition" bson:"condition" mapstructure:"condition,omitempty"`

// internal idempotency key for a create operation.
Expand Down Expand Up @@ -681,10 +684,6 @@ func (o *Automation) Validate() error {
errors = errors.Append(err)
}

if err := elemental.ValidateRequiredString("condition", o.Condition); err != nil {
requiredErrors = requiredErrors.Append(err)
}

if err := elemental.ValidateMaximumLength("description", o.Description, 1024, false); err != nil {
errors = errors.Append(err)
}
Expand Down Expand Up @@ -856,13 +855,15 @@ var AutomationAttributesMap = map[string]elemental.AttributeSpecification{
"Condition": elemental.AttributeSpecification{
AllowedChoices: []string{},
ConvertedName: "Condition",
Description: `Condition contains the code that will be executed to decide if any action should
be taken.`,
Exposed: true,
Name: "condition",
Required: true,
Stored: true,
Type: "string",
Description: `Condition contains the code that will be executed to decide if any action(s)
should be executed. Providing a condition for an automation with a
"Webhook" trigger type will have no impact as the condition will not be
evaluated. If no condition is defined, then the automation action(s) will be executed;
this behaves akin to a condition that always succeeds.`,
Exposed: true,
Name: "condition",
Stored: true,
Type: "string",
},
"CreateIdempotencyKey": elemental.AttributeSpecification{
AllowedChoices: []string{},
Expand Down Expand Up @@ -1209,13 +1210,15 @@ var AutomationLowerCaseAttributesMap = map[string]elemental.AttributeSpecificati
"condition": elemental.AttributeSpecification{
AllowedChoices: []string{},
ConvertedName: "Condition",
Description: `Condition contains the code that will be executed to decide if any action should
be taken.`,
Exposed: true,
Name: "condition",
Required: true,
Stored: true,
Type: "string",
Description: `Condition contains the code that will be executed to decide if any action(s)
should be executed. Providing a condition for an automation with a
"Webhook" trigger type will have no impact as the condition will not be
evaluated. If no condition is defined, then the automation action(s) will be executed;
this behaves akin to a condition that always succeeds.`,
Exposed: true,
Name: "condition",
Stored: true,
Type: "string",
},
"createidempotencykey": elemental.AttributeSpecification{
AllowedChoices: []string{},
Expand Down Expand Up @@ -1587,8 +1590,11 @@ type SparseAutomation struct {
// List of tags attached to an entity.
AssociatedTags *[]string `json:"associatedTags,omitempty" msgpack:"associatedTags,omitempty" bson:"associatedtags,omitempty" mapstructure:"associatedTags,omitempty"`

// Condition contains the code that will be executed to decide if any action should
// be taken.
// Condition contains the code that will be executed to decide if any action(s)
// should be executed. Providing a condition for an automation with a
// "Webhook" trigger type will have no impact as the condition will not be
// evaluated. If no condition is defined, then the automation action(s) will be executed;
// this behaves akin to a condition that always succeeds.
Condition *string `json:"condition,omitempty" msgpack:"condition,omitempty" bson:"condition,omitempty" mapstructure:"condition,omitempty"`

// internal idempotency key for a create operation.
Expand Down
3 changes: 2 additions & 1 deletion custom_validations.go
View file
Open in desktop
Expand Up @@ -417,7 +417,8 @@ func ValidateHTTPMethods(attribute string, methods []string) error {
return nil
}

// ValidateAutomation validates an automation
// ValidateAutomation validates an automation by checking for the following:
// - Exactly one action MUST be defined if the automation trigger type is set to "Webhook"
func ValidateAutomation(auto *Automation) error {
switch auto.Trigger {
case AutomationTriggerWebhook:
Expand Down
26 changes: 14 additions & 12 deletions custom_validations_test.go
View file
Open in desktop
Expand Up @@ -913,14 +913,19 @@ func TestValidateOptionalNetworkList(t *testing.T) {
}

func TestValidateAutomation(t *testing.T) {
testCases := []struct {
scenario string
testCases := map[string]struct {
automation *Automation
shouldError bool
}{
{
scenario: "should not return an error if trigger type is not webhook and multiple actions have been defined",
"should not return an error if trigger type is not webhook and multiple actions have been defined": {
automation: &Automation{
Condition: `
function when(api, params) {
return {
continue: true,
};
}
`,
Trigger: AutomationTriggerRemoteCall,
Actions: []string{
"Action 1",
Expand All @@ -929,8 +934,7 @@ func TestValidateAutomation(t *testing.T) {
},
shouldError: false,
},
{
scenario: "should not return an error if trigger type is webhook and one action has been defined",
"should not return an error if trigger type is webhook and one action has been defined": {
automation: &Automation{
Trigger: AutomationTriggerWebhook,
Actions: []string{
Expand All @@ -939,8 +943,7 @@ func TestValidateAutomation(t *testing.T) {
},
shouldError: false,
},
{
scenario: "should return an error if trigger type is set to webhook and more than one action has been defined",
"should return an error if trigger type is set to webhook and more than one action has been defined": {
automation: &Automation{
Trigger: AutomationTriggerWebhook,
Actions: []string{
Expand All @@ -950,8 +953,7 @@ func TestValidateAutomation(t *testing.T) {
},
shouldError: true,
},
{
scenario: "should return an error if trigger type is set to webhook and no actions have been defined",
"should return an error if trigger type is set to webhook and no actions have been defined": {
automation: &Automation{
Trigger: AutomationTriggerWebhook,
Actions: nil,
Expand All @@ -960,8 +962,8 @@ func TestValidateAutomation(t *testing.T) {
},
}

for _, tc := range testCases {
t.Run(tc.scenario, func(t *testing.T) {
for scenario, tc := range testCases {
t.Run(scenario, func(t *testing.T) {
err := ValidateAutomation(tc.automation)
switch {
case err != nil && tc.shouldError:
Expand Down
9 changes: 6 additions & 3 deletions doc/documentation.md
View file
Open in desktop
Expand Up @@ -4996,10 +4996,13 @@ Stores additional information about an entity.

List of tags attached to an entity.

##### `condition` `string` [`required`]
##### `condition` `string`

Condition contains the code that will be executed to decide if any action should
be taken.
Condition contains the code that will be executed to decide if any action(s)
should be executed. Providing a condition for an automation with a
"Webhook" trigger type will have no impact as the condition will not be
evaluated. If no condition is defined, then the automation action(s) will be executed;
this behaves akin to a condition that always succeeds.

##### `createTime` `time` [`autogenerated`,`read_only`]

Expand Down
8 changes: 5 additions & 3 deletions specs/automation.spec
View file
Open in desktop
Expand Up @@ -44,12 +44,14 @@ attributes:

- name: condition
description: |-
Condition contains the code that will be executed to decide if any action should
be taken.
Condition contains the code that will be executed to decide if any action(s)
should be executed. Providing a condition for an automation with a
"Webhook" trigger type will have no impact as the condition will not be
evaluated. If no condition is defined, then the automation action(s) will be
executed; this behaves akin to a condition that always succeeds.
type: string
exposed: true
stored: true
required: true
example_value: 'function when(m, params) { return { continue: true }}'

- name: entitlements
Expand Down