Skip to content

Conversation

@camrossi
Copy link

  • Add external_subnet_selectors to endpoint security groups
  • Support for external subnet selection in ESG configuration
  • Update documentation and examples for external subnet selectors
  • Include validation and proper variable definitions

- Add external_subnet_selectors to endpoint security groups
- Support for external subnet selection in ESG configuration
- Update documentation and examples for external subnet selectors
- Include validation and proper variable definitions
@robvand
Copy link
Contributor

robvand commented Aug 18, 2025

Few remarks:

The shared bool is not idempotent:

  # module.aci.module.aci_endpoint_security_group["ts-ocp-1/openshift/egress"].aci_rest_managed.fvExternalSubnetSelector["10.61.6.80/28"] will be updated in-place
  ~ resource "aci_rest_managed" "fvExternalSubnetSelector" {
      ~ content     = {
          ~ "shared" = "yes" -> "true"
            # (1 unchanged element hidden)
        }
        id          = "uni/tn-ts-ocp-1/ap-openshift/esg-egress/extsubselector-[10.61.6.80/28]"
        # (4 unchanged attributes hidden)

Value key should probably be ip: for consistency.

@camrossi
Copy link
Author

@robvand fixed it
However I only changed from value to ip in the ip_external_subnet_selectors. value was already used in all the other instances so I do not wanna introduce any breaking change. Perhaps a different PR should address this for ip_subnet_selectors

@robvand
Copy link
Contributor

robvand commented Aug 19, 2025

Thanks for the fixes. Seems your PR includes some changes to the set-rule module which should be omitted.

@juchowan juchowan linked an issue Sep 25, 2025 that may be closed by this pull request
@jmmarquezg
Copy link
Contributor

The internal repo is using "value" instead of "ip" for that attribute. I made some changes and pushed them to this very same PR.
Nothing wrong with the code logic, just to keep consistency across all repos.
I might need you to allow me to send changes to your fork so this changes can be made in this same fork.

@camrossi
Copy link
Author

camrossi commented Nov 6, 2025

@jmmarquezg I used ip because @robvand pointed out that value is not a very good name for this.
I agreed with Rob and was thinking for values that are ips we should indeed use ip or at least add it as an alias so we can keep backward compatibility perhaps?

@juchowan
Copy link
Contributor

juchowan commented Nov 12, 2025

Let's keep it as ip. @jmmarquezg please update the internal repo

@jmmarquezg
Copy link
Contributor

Internal repo now is matching "ip" parameter name

@juchowan
Copy link
Contributor

@jmmarquezg still pending updates in this PR

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.

Enhancement: Support for esg_external_subnet_selectors

4 participants