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

Modify security group ingress rules to allow access only on specific ports #105

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

gaiksaya
Copy link
Member

@gaiksaya gaiksaya commented Feb 17, 2024

Description

In order to allow access only on specific ports of the cluster, this change modifies the tcp port access of security groups from alltcp to 80, 443, 9200, 5601 and 8443.

Since this is a breaking change, bumping the version to next major.

Issues Resolved

related opensearch-project/opensearch-devops#129

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Feb 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.15%. Comparing base (b4360fc) to head (9de23a5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
+ Coverage   82.00%   82.15%   +0.15%     
==========================================
  Files           6        6              
  Lines         450      454       +4     
  Branches      169      169              
==========================================
+ Hits          369      373       +4     
  Misses         81       81              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gaiksaya gaiksaya changed the title Modify security group ingress rules to allow only access on specific ports Modify security group ingress rules to allow access only on specific ports Feb 19, 2024
@rishabh6788
Copy link
Collaborator

Little confused here.
The purpose of the accepting securityGroupId parameter was that if the user wants to reuse an existing security group they can provide that and the access restrictions on the ec2 hosts will based on that. This security group will the one attached to ec2 instances.
It assumed that the provided SG would reside in the same VPC, but it is good to enforce a failure if only SG-id was provided and not VPC.

Coming to serverAccessType , the purpose of introducing these settings was to add rules to an existing security group. So when I provide serverAccessType=securityGroupId, it meant that I am adding a new rule in an existing security group (attached to ec2 instances) that restricts the access from resources behind the security group provided in serverAccessType.

I would like to keep them both separated.

@gaiksaya
Copy link
Member Author

Little confused here. The purpose of the accepting securityGroupId parameter was that if the user wants to reuse an existing security group they can provide that and the access restrictions on the ec2 hosts will based on that. This security group will the one attached to ec2 instances. It assumed that the provided SG would reside in the same VPC, but it is good to enforce a failure if only SG-id was provided and not VPC.

Coming to serverAccessType , the purpose of introducing these settings was to add rules to an existing security group. So when I provide serverAccessType=securityGroupId, it meant that I am adding a new rule in an existing security group (attached to ec2 instances) that restricts the access from resources behind the security group provided in serverAccessType.

I would like to keep them both separated.

I see what you mean. I will update this PR to only reflect the specific port access instead of all ports for now. Will create another PR for security Group ID change and throwing an error if vpc id was not provided.

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@gaiksaya gaiksaya merged commit ca54337 into opensearch-project:main Mar 12, 2024
5 checks passed
@gaiksaya gaiksaya deleted the add-certs branch March 12, 2024 22:56
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.

2 participants