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

Create How to guide for security groups #142

Merged
merged 11 commits into from
Apr 2, 2024
Merged

Conversation

josephineSei
Copy link
Contributor

Instead of a standard, the security groups just allow a guide for best practice including basic recommended security groups.

This is part of the ticket: SovereignCloudStack/standards#473

Add the guide plus best practices groups.

Signed-off-by: josephineSei <[email protected]>
@josephineSei josephineSei self-assigned this Feb 19, 2024
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
@markus-hentsch
Copy link
Contributor

Did a first review, see review comments.

Things I didn't address yet:

  1. We should add a warning section/banner early in the document stating that security groups can be edited at runtime affecting all ports/VMs they are assigned to immediately. SCS should make users aware of this as I think this is a very important detail that may not be immediately obvious.
  2. In many paragraphs "Ports" is written with a capital "P", most likely to differentiate Neutron Ports from actual network ports (numbers or NICs). However, in contrast "security groups" is using lower case across the document but also refers to a special concept of OpenStack not identical to the linguistic interpretation of the word combination "security groups".
  3. I'm wondering if we should recommend using security groups as a grouping feature primarily. I.e. not creating individual groups for HTTP, HTTPS, ICMP etc. but actually bringing the "groups" concept into play by suggesting creating abstract service groups, for example:
    • admin group: ICMP, SSH port
    • webserver group: HTTP, HTTPS ports
    • infrastructure group: NTP, DNS ports

fix some spelling mistakes and the wording of the introduction

Co-authored-by: Markus Hentsch <[email protected]>
Signed-off-by: josephineSei <[email protected]>
@josephineSei
Copy link
Contributor Author

Did a first review, see review comments.

Things I didn't address yet:

1. We should add a warning section/banner early in the document stating that security groups can be edited at runtime _affecting all ports/VMs they are assigned to immediately_. SCS should make users aware of this as I think this is a very important detail that may not be immediately obvious.

This is a good idea.

2. In many paragraphs "Ports" is written with a capital "P", most likely to differentiate Neutron Ports from actual network ports (numbers or NICs). However, in contrast "security groups" is using lower case across the document but also refers to a special concept of OpenStack not identical to the linguistic interpretation of the word combination "security groups".

I wonder, does the documentation already has some internal "standard" for this? Because it is important and I would from now on continue with writing all OpenStack resources with capital letters: Ports, Security Groups, Volumes, Server / VM, Network, etc..

3. I'm wondering if we should recommend using security groups as a grouping feature primarily. I.e. not creating individual groups for HTTP, HTTPS, ICMP etc. but actually bringing the "groups" concept into play by suggesting creating abstract service groups, for example:
   
   * admin group: ICMP, SSH port
   * webserver group: HTTP, HTTPS ports
   * infrastructure group: NTP, DNS ports

I was also thinking about this. But after looking into the default quotas for Security Groups (that is 10 btw) I am a bit afraid that we would recommend to many groups. But maybe we should recommend different ways? Like:

  • you could either use the protocol-based SGs
  • or you could user the conceptual SGs

Copy link
Contributor

@markus-hentsch markus-hentsch left a comment

Choose a reason for hiding this comment

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

Very good additions!

I added some minor spelling and phrasing improvement suggestions.

I also changed all occurences of "Security Group(s)" to lowercase to align it with the existing paragraphs.

One thing I'm wondering is how we should approach the spelling of "VM" vs "virtual machine". Currently, we seem to use both.

docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
docs/02-iaas/security-groups.md Outdated Show resolved Hide resolved
Co-authored-by: Markus Hentsch <[email protected]>
Signed-off-by: josephineSei <[email protected]>
@josephineSei
Copy link
Contributor Author

@markus-hentsch I added your suggestions.
I am not sure right now whether we should keep "security groups" lower case or use capital letters to indicate, that it is an OpenStack resource we are referring.

@artificial-intelligence and @bitkeks could you please look over this guide and state, whether this is security-focused enough? You both indicated, that you wanted to have a more security-focused guide in the DR.

Signed-off-by: josephineSei <[email protected]>
Co-authored-by: Markus Hentsch <[email protected]>
Signed-off-by: josephineSei <[email protected]>
@berendt
Copy link
Member

berendt commented Apr 2, 2024

Will merge this after final CI run

@berendt berendt merged commit 35313fc into main Apr 2, 2024
4 checks passed
@berendt berendt deleted the how-to-security-groups branch April 2, 2024 17:20
Copy link
Contributor

@artificial-intelligence artificial-intelligence left a comment

Choose a reason for hiding this comment

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

I was asked to review this, after it's already merged.
It's mostly looking good to me, some minor nits could be improved, but that should be discussed in a version 2 of this document imho.

JuanPTM pushed a commit to JuanPTM/docs that referenced this pull request Jun 3, 2024
Add the guide plus best practices groups.

Signed-off-by: josephineSei <[email protected]>
Co-authored-by: Markus Hentsch <[email protected]>
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.

4 participants