Skip to content

Rework API fields for nodegroup SGs#460

Merged
errordeveloper merged 3 commits into
masterfrom
fix-425
Jan 25, 2019
Merged

Rework API fields for nodegroup SGs#460
errordeveloper merged 3 commits into
masterfrom
fix-425

Conversation

@errordeveloper
Copy link
Copy Markdown
Contributor

@errordeveloper errordeveloper commented Jan 24, 2019

Description

Checklist

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All tests passing (i.e. make test)
  • Added/modified documentation as required (such as the README)
  • Added yourself to the humans.txt file

@errordeveloper
Copy link
Copy Markdown
Contributor Author

@dlespiau this is still WIP, but I'd like to hear your feedback on the API change itself.

@errordeveloper errordeveloper changed the title WIP: Remove API fields for nodegroup SGs WIP: Rework API fields for nodegroup SGs Jan 24, 2019
@errordeveloper
Copy link
Copy Markdown
Contributor Author

@rndstr FYI

@dlespiau
Copy link
Copy Markdown
Contributor

I guess it's to allow disabling the default SGs. Seems fine to me.

@errordeveloper
Copy link
Copy Markdown
Contributor Author

errordeveloper commented Jan 24, 2019 via email

@errordeveloper
Copy link
Copy Markdown
Contributor Author

Will also need to address #465.

@errordeveloper
Copy link
Copy Markdown
Contributor Author

I think it'd be good to also amend nodegroup check function to detect clusters that were affected by #465 and give user a warning.

@errordeveloper errordeveloper force-pushed the fix-425 branch 3 times, most recently from 38b6ee9 to 47268f5 Compare January 25, 2019 12:12
@errordeveloper errordeveloper changed the title WIP: Rework API fields for nodegroup SGs Rework API fields for nodegroup SGs Jan 25, 2019
@errordeveloper errordeveloper merged commit c97f45a into master Jan 25, 2019
@errordeveloper errordeveloper deleted the fix-425 branch January 25, 2019 14:51
torredil pushed a commit to torredil/eksctl that referenced this pull request May 20, 2022
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.

shared security group is disabled by default when using config file ensure all config fields that can be set are handled appropriately

2 participants