Liblor / applied_sec_lab

Applied Security Laboratory - AS19
6 stars 1 forks source link

Remove vagrant #51

Closed Miro-H closed 5 years ago

Miro-H commented 5 years ago
keyctl commented 5 years ago

I'll have a look at this in the evening.

Miro-H commented 5 years ago

I'm working on resolving the merge conflicts

Miro-H commented 5 years ago

Merge is done, I will build a new architecture later during dinner.

By the way, to test the vagrant purge feature, use PURGE_VAGRANT="true" vagrant up (or uncomment this in the Makefile)

Miro-H commented 5 years ago

Merge is done, I will build a new architecture later during dinner.

By the way, to test the vagrant purge feature, use PURGE_VAGRANT="true" vagrant up (or uncomment this in the Makefile)

Merge worked for me. But @keyctl, please check that I didn't revert something unintentionally.

Miro-H commented 5 years ago

TODO: Unmounting the shared folders is not enough, vagrant brings them back when starting the VMs again. Check if we can stop this.

keyctl commented 5 years ago

This patch addresses #40.

keyctl commented 5 years ago

TODO: Unmounting the shared folders is not enough, vagrant brings them back when starting the VMs again. Check if we can stop this.

I think restarting unmounts them correctly.

keyctl commented 5 years ago

The normal build works for me without errors on this branch.

Miro-H commented 5 years ago

TODO: Unmounting the shared folders is not enough, vagrant brings them back when starting the VMs again. Check if we can stop this.

I think restarting unmounts them correctly.

But if I do vagrant halt && vagrant up the shared folders will be installed again I think. They also stay in the VirtualBox settings for me. (I have to still test that specifically, that's just what I noticed while doing other stuff)

Miro-H commented 5 years ago

It would be good if we could merge this to the master soon, because every change to the Vagrantfile on the master will cause ugly merge conflicts until we do so.

keyctl commented 5 years ago

It would be good if we could merge this to the master soon, because every change to the Vagrantfile on the master will cause ugly merge conflicts until we do so.

I gave up testing since you said it didn't work. Should I test once more?

Is this branch feature complete?

Miro-H commented 5 years ago

It would be good if we could merge this to the master soon, because every change to the Vagrantfile on the master will cause ugly merge conflicts until we do so.

I gave up testing since you said it didn't work. Should I test once more?

Is this branch feature complete?

It works, just when we continue to use vagrants for the VMs, I think it will reinstall shared folders. But we can add this as an improvement later. You can test and merge if it works.

keyctl commented 5 years ago

I will build with BUILD_TYPE=1 now and notify you when done. If it works, we can merge.

Have you checked if the systems work when Vagrant is removed?

keyctl commented 5 years ago

I cannot confirm if the machines work since I could not find the SSH key to the ansible master. When visiting https://aslweb01/, nginx returns an error.

Miro-H commented 5 years ago

I cannot confirm if the machines work since I could not find the SSH key to the ansible master. When visiting https://aslweb01/, nginx returns an error.

In that case it probably worked :) You can only connect to the machines with the remote administrator. There is an account on aslclient01 called admin (pw admin) and it has the ssh keys installed to access aslans01 in ~/.ssh (they have a special name, so they must be added explicitly with -i). The vagrant script also uses this method to run the ansible script that purges vagrant.

Miro-H commented 5 years ago

I cannot confirm if the machines work since I could not find the SSH key to the ansible master. When visiting https://aslweb01/, nginx returns an error.

Also the error this should only be because this is an older branch and changes to the server were not pushed to the master yet, so it still installs an older release by default

keyctl commented 5 years ago

@Miro-H I added a few more remarks. If you help me resolve these, I can merge this.

I've tested this in both modes. For the Vagrant-less version, I've powered off all machines, and started them headless using Virtualbox. I could login to aslans* using the user admin. Other machines were running, too.

Miro-H commented 5 years ago

@Miro-H I added a few more remarks. If you help me resolve these, I can merge this.

I've tested this in both modes. For the Vagrant-less version, I've powered off all machines, and started them headless using Virtualbox. I could login to aslans* using the user admin. Other machines were running, too.

I will address those remarks later tonight.

Miro-H commented 5 years ago

I applied the changes that were requested, once again resolved the merge conflicts and tested it with a fresh build (only the vagrant purge option, but the other one should be implied).

keyctl commented 5 years ago

Wouldn't this be generally much cleaner if we added a Make target for removing Vagrant from the machines? Like in #60, we'd have a script that's uploaded to the Ansible master that cleans Vagrant from all other machines and itself?

keyctl commented 5 years ago

It would be kind of neat to be able to remove Vagrant from a build you just tested, but I'm not sure if that's possible.

Miro-H commented 5 years ago

Wouldn't this be generally much cleaner if we added a Make target for removing Vagrant from the machines? Like in #60, we'd have a script that's uploaded to the Ansible master that cleans Vagrant from all other machines and itself?

That wouldn't work since you cannot remove the vagrant user of the ansible server when it is used for the SSH connection. We'd also have to do the indirection over the admin user of aslclient*. That could work, would maybe be a little cleaner but I'm not sure it's worth it. We already spent enough time here.

Also removing vagrant from a build you just tested should work. Either you run ansible with the cleanup flag yourself or you run vagrant up aslclient* and it should trigger the vagrant purge as well.

keyctl commented 5 years ago

That wouldn't work since you cannot remove the vagrant user of the ansible server when it is used for the SSH connection. We'd also have to do the indirection over the admin user of aslclient*. That could work, would maybe be a little cleaner but I'm not sure it's worth it. We already spent enough time here.

We could trigger a task that decouples from the SSH connection, I think that's not even that bad of an architecture. But you're right, let's merge this finally.

Also removing vagrant from a build you just tested should work. Either you run ansible with the cleanup flag yourself or you run vagrant up aslclient* and it should trigger the vagrant purge as well.

Yes, but then you'd need to do that through the client again, using the admin user. That's times more complex to do, so I'd prefer a simple command out of the Git root.

Miro-H commented 5 years ago

Yes, but then you'd need to do that through the client again, using the admin user. That's times more complex to do, so I'd prefer a simple command out of the Git root.

PURGE_VAGRANT="true" vagrant up aslclient01 could work from the Git root after the first provisioning. Haven't tested that though.

@keyctl Can you test one last time the latest changes and then we merge?

keyctl commented 5 years ago

@Miro-H Builds correctly for both options. Ready to merge.