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

feat: Adding LDAP into rules.tf #195

Merged
merged 5 commits into from
Nov 13, 2020
Merged

feat: Adding LDAP into rules.tf #195

merged 5 commits into from
Nov 13, 2020

Conversation

lazzurs
Copy link
Contributor

@lazzurs lazzurs commented Nov 9, 2020

Adding LDAP along side LDAPS as LDAP without encryption is still used in places.

Given the rules.tf has both HTTP and HTTPS this appears to be a valid addition.

Description

Adding TCP 389 LDAP to rules.tf

Motivation and Context

Adding the default LDAP port to the rules.

Breaking Changes

No breaking changes.

How Has This Been Tested?

No tests have been run of this change.

Adding LDAP along side LDAPS as LDAP without encryption is still used in places.

Given the rules.tf has both HTTP and HTTPS this appears to be a valid addition.
@lazzurs lazzurs changed the title Adding LDAP into rules.tf feat: Adding LDAP into rules.tf Nov 9, 2020
lazzurs and others added 4 commits November 9, 2020 15:17
Adding LDAP along side LDAPS as LDAP without encryption is still used in places.

Given the rules.tf has both HTTP and HTTPS this appears to be a valid addition.
@antonbabenko antonbabenko merged commit 0c0572f into terraform-aws-modules:master Nov 13, 2020
@antonbabenko
Copy link
Member

Thank you!

v3.17.0 has been just released.

@lazzurs lazzurs deleted the feature/add_ldap_rules branch November 19, 2020 15:33
@Porkepix
Copy link

Porkepix commented Jul 2, 2021

@lazzurs @antonbabenko It seems 0c0572f#diff-dfd5848fa77a04d0343891fe859286cc210473d10f0e62c7f99f0d5ecf1a9927 reverted what was done in #184 allowing to use provider v3 reverting to a state where provider is locked to version 2 only.
Is there any reason for this?

@antonbabenko
Copy link
Member

@Porkepix I am not sure which version you are referring to. The latest version of this module (4.3.0) allows using the correct range of Terraform AWS providers (>=2.42) and the correct version of Terraform (>=0.12.6).

https://github.com/terraform-aws-modules/terraform-aws-security-group/blob/master/versions.tf

@Porkepix
Copy link

Porkepix commented Jul 7, 2021

@antonbabenko What I mean by that is that PR #184 introduced this version restriction aws = ">= 2.42, < 4.0" and the commit I point to, while only talking about LDAP related change reverted it back to aws = ">= 2.42" notation, so I was curious as about why it did so.

This can come from a misunderstanding on my end though, it appears that I though I saw some of our layers still picking the provider v3 due to that, but I guess it might be because of the first apply after module upgrade and that it still used the provider known in the state file.

@antonbabenko
Copy link
Member

Provider version is not tracked in terraform state, but terraform-aws-provider currently is v3.49.0 that is falling inside the specified range >= 2.42. The upper bound of the range (< 4.0) is almost never necessary in Terraform AWS modules.

If you want to force an upgrade of the version of the providers you can run terraform init -upgrade=true.

@Porkepix
Copy link

Porkepix commented Jul 7, 2021

Provider version is not tracked in terraform state, but terraform-aws-provider currently is v3.49.0 that is falling inside the specified range >= 2.42. The upper bound of the range (< 4.0) is almost never necessary in Terraform AWS modules.

If you want to force an upgrade of the version of the providers you can run terraform init -upgrade=true.

Yup I always do terraform init -upgrade, though I think I got some times where it was still using provider v2. Let's assume it's a mistake on my end and/or due to the different modules such as this one (and a couple of others) I was upgrading. So, sorry for the noise here.

PS: When I was migrating terraform versions, I got situations where it was using both an old and a new provider to satisfy to such constraints, and I believe some of them were coming from the state file. I can obviously be wrong here.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants