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

Adding support for unidirectional contracts #95

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

Conversation

robvand
Copy link
Contributor

@robvand robvand commented May 30, 2024

Adding support for unidirectional contracts in the aci_contracts module.

Modified subject attributes to include:
reverse_filter_ports

consumer_to_provider_service_graph
consumer_to_provider_qos_class
consumer_to_provider_target_dscp
consumer_to_provider_filters

provider_to_consumer_service_graph
provider_to_consumer_qos_class
provider_to_consumer_target_dscp
provider_to_consumer_filters

Directionally derived from presence of consumer_to_provider or provider_to_consumer_filters.

@robvand
Copy link
Contributor Author

robvand commented May 30, 2024

closes #94

Copy link
Contributor

@andbyrne andbyrne left a comment

Choose a reason for hiding this comment

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

My review is out of date. I will re-review once the code has been updated to align with final schema decisions.

aci_tenants.tf Outdated Show resolved Hide resolved
aci_tenants.tf Outdated Show resolved Hide resolved
aci_tenants.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
@andbyrne andbyrne linked an issue Jun 21, 2024 that may be closed by this pull request
defaults/defaults.yaml Outdated Show resolved Hide resolved
defaults/defaults.yaml Outdated Show resolved Hide resolved
modules/terraform-aci-contract/examples/complete/main.tf Outdated Show resolved Hide resolved
aci_tenants.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
@danischm
Copy link
Member

Please merge the latest commits from main, which should fix the CI workflow.

@andbyrne andbyrne self-requested a review July 11, 2024 23:31
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/main.tf Outdated Show resolved Hide resolved
@@ -73,23 +73,43 @@ variable "target_dscp" {
}
}


variable "subjects" {
description = "List of contract subjects. Choices `action`: `permit`, `deny`. Default value `action`: `permit`. Choices `priority`: `default`, `level1`, `level2`, `level3`. Default value `priority`: `default`. Default value `log`: `false`. Default value `no_stats`: `false`. Choices `qos_class`: `unspecified`, `level1`, `level2`, `level3`, `level4`, `level5` or`level6`. Default value `qos_class`: `unspecified`. Choices `dscp_target` : `unspecified`, `CS0`, `CS1`, `AF11`, `AF12`, `AF13`, `CS2`, `AF21`, `AF22`, `AF23`, `CS4`, `AF41`, `AF42`, `AF43`, `CS5`, `VA`, `EF`, `CS6` `CS7` or a number between 0 and 63. Default value `dscp_target`: `unspecified`"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add:

Default value `reverse_filter_ports`: `true`.

modules/terraform-aci-contract/variables.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/variables.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/variables.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/variables.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/variables.tf Outdated Show resolved Hide resolved
modules/terraform-aci-contract/variables.tf Show resolved Hide resolved
modules/terraform-aci-contract/variables.tf Outdated Show resolved Hide resolved
@robvand robvand force-pushed the 311 branch 3 times, most recently from a3404bd to cb69588 Compare July 17, 2024 07:53
Copy link
Contributor

@andbyrne andbyrne left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: Need support for uni-directional contracts
3 participants