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

Allow VM instance security group modification #118

Merged
merged 6 commits into from
Mar 26, 2019
Merged

Conversation

falzm
Copy link
Contributor

@falzm falzm commented Mar 11, 2019

Fixes #113.

Example usage:

$ ./exo vm show vm1
┼───────────────────────┼──────────────────────────────────────┼
│          VM1          │                                      │
┼───────────────────────┼──────────────────────────────────────┼
│ State                 │ Stopped                              │
│ OS Template           │ Linux Ubuntu 18.04 LTS 64-bit        │
│ Region                │ ch-dk-2                              │
│ Instance Type         │ Tiny                                 │
│ Disk                  │ 10 GB                                │
│ Instance Hostname     │ vm1                                  │
│ Instance Display Name │ vm1                                  │
│ Instance Username     │ ubuntu                               │
│ Created on            │ 2019-03-08T11:25:12+0100             │
│ Base SSH Key          │ default                              │
│ Security Groups       │ default                              │
│ Affinity Groups       │                                      │
│ Instance IP           │ a.b.c.d                              │
│ ID                    │ 1ed46c21-b32d-4359-bd38-d0d149adbe09 │
┼───────────────────────┼──────────────────────────────────────┼

$ ./exo vm firewall add vm1 sg1 sg2
┼─────────────────┼─────────────────────┼
│       VM1       │                     │
┼─────────────────┼─────────────────────┼
│ Security Groups │ default - sg1 - sg2 │
┼─────────────────┼─────────────────────┼

$ ./exo vm firewall remove vm1 sg2
┼─────────────────┼───────────────┼
│       VM1       │               │
┼─────────────────┼───────────────┼
│ Security Groups │ default - sg1 │
┼─────────────────┼───────────────┼

$ ./exo vm firewall set vm1 default
┼─────────────────┼─────────┼
│       VM1       │         │
┼─────────────────┼─────────┼
│ Security Groups │ default │
┼─────────────────┼─────────┼

@falzm falzm changed the title Allow VM instance security group modification with exo CLI Allow VM instance security group modification Mar 11, 2019
@falzm
Copy link
Contributor Author

falzm commented Mar 11, 2019

@mcorbin
Copy link
Contributor

mcorbin commented Mar 12, 2019

I have an issue with the remove subcommand. It doesn't seem to work when I try to delete multiple security groups:

$ ./exo vm firewall set debian-build ssh default
┼─────────────────┼───────────────┼
│  DEBIAN-BUILD   │               │
┼─────────────────┼───────────────┼
│ Security Groups │ default - ssh │
┼─────────────────┼───────────────┼

./exo vm firewall remove debian-build ssh default
┼─────────────────┼───────────────┼
│  DEBIAN-BUILD   │               │
┼─────────────────┼───────────────┼
│ Security Groups │ default - ssh │
┼─────────────────┼───────────────┼

./exo vm firewall remove debian-build ssh
┼─────────────────┼─────────┼
│  DEBIAN-BUILD   │         │
┼─────────────────┼─────────┼
│ Security Groups │ default │
┼─────────────────┼─────────┼

@falzm
Copy link
Contributor Author

falzm commented Mar 12, 2019

I managed to reproduce your issue, however I'm not sure I have a solution for this since it is a consequence of the Go JSON serialization with the omitempty tag: if one removes all the current security groups of an instance, the egoscale.UpdateVirtualMachine.SecurityGroupIDs struct field serializes the empty []egoscale.UUID slice value with omitempty field tag set, which amounts to performing a updateVirtualMachine CloudStack API call without modifying the securitygroupids parameter, hence the instance SG remaining the same as they are currently set.

Maybe @greut has some 🎩 🐇 ✨ for that...

cmd/vm_firewall.go Outdated Show resolved Hide resolved
Copy link
Contributor

@greut greut left a comment

Choose a reason for hiding this comment

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

thanks @falzm 👍

greut and others added 2 commits March 25, 2019 08:49
cmd/vm_firewall.go Outdated Show resolved Hide resolved
cmd/vm_firewall.go Outdated Show resolved Hide resolved
cmd/vm_firewall.go Outdated Show resolved Hide resolved
Copy link
Contributor

@greut greut left a comment

Choose a reason for hiding this comment

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

🍯

@falzm falzm merged commit 0ea4d67 into master Mar 26, 2019
@falzm falzm deleted the feature/vm-firewall branch March 26, 2019 09:54
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.

None yet

3 participants