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

Lock CustomFile and custom main provisioners behind a flag #2571

Open
tomjn opened this issue Jan 10, 2022 · 7 comments
Open

Lock CustomFile and custom main provisioners behind a flag #2571

tomjn opened this issue Jan 10, 2022 · 7 comments

Comments

@tomjn
Copy link
Member

tomjn commented Jan 10, 2022

Describe alternatives you've considered

We should follow composers lead and lock these features behind a config.yml flag so that they have to be explicitly turned on to work. Specifically, legacy features to replace the main provisioner or add custom hooks on triggers such as CustomFile, provision-custom.sh in the main provision folder, pre-provision.sh, files such as vagrant_up_custom etc in the homebin folder.

Additional context

Support for these features is only present for legacy reasons and because people are still using them. Removing them would require a VVV 4.0, but until then this should be dissuaded.

Ideally, VVV would notice these files exist, and show a clear warning message indicating that it found the file but will not use the file because permission has not been given, and that support is deprecated and going away in a future major version.

@tomjn
Copy link
Member Author

tomjn commented Jan 10, 2022

Perhaps something like this:

deprecated:
  enable_customfile: false
  enable_vagrant_custom: false
  enable_pre_provision: false
  enable_provision_custom: false

@tomjn
Copy link
Member Author

tomjn commented Jan 10, 2022

There are also per site custom files, I don't think this particular method is optimal for those, we'll want to circleback and evaluate how effective this is before touching that

@Mte90
Copy link
Member

Mte90 commented Jan 17, 2022

So a first analysis on how to do the various implementations for tracking purpose:

Customfile for provisioners

On https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/Vagrantfile#L153 there is a flag for this, we need to expose this in the config.yml

Customfile for vagrant

The lines that involve this are https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/Vagrantfile#L677

Pre provisioners

To run before all the provisioners it's the case to do there https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/provision/provision.sh#L49

Pre provision site

It's the case to act there after updating the git repository https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/provision/provision-site.sh#L481

After provision site

Same file above but at the end before the file ending

@tomjn
Copy link
Member Author

tomjn commented Jan 18, 2022

@Mte90 it may be that we've deliberately not documented site provisioner customfiles to dissuade people from using them. I'm not a fan of undoing that. In this case the documentation would be VVV telling you in the terminal in big yellow letters that it's turned off, deprecated, and you have to turn it back on by doing X Y Z, and a mention in the changelog that these options now have to be enabled. The last thing we want is to encourage people to do this.

@Mte90
Copy link
Member

Mte90 commented Jan 18, 2022

As today for customfile there is already an alert so should be enough write in a different color. Also create the new parameter to remap to the old one so we keep the compatibility.

In the meantime I will work on the other part that is more easy about what to do.

I was thinking as parameters:

general:
   custom:
       customfile: false
       pre_provision: [path/to/sh]
       pre_provisioners: [path/to/sh]
       post_provision: [path/to/sh]

As I think that let users to execute stuff before/after provision should be something not legacy but part of the tool to adapt VVV to their usage.

@tomjn
Copy link
Member Author

tomjn commented Jan 21, 2022

That would introduce new features though, we're trying to make it easier to get rid of these features and dissuade people from using them. They were put behind a deprecated: section to further reinforce that these are going away in the future and are only there for legacy reasons.

I know there's a very particular thing that you want to do in a post_provision file, but this is not the way to do it. You've proposed an X Y problem. Instead raise a new ticket for that, and keep in mind the thing you want to do is itself not a problem, but a proposed solution to another problem you have. If the phrasing of it can be worded as "how can I do this thing that fixes this problem" then you need to go one step further down.

@tomjn
Copy link
Member Author

tomjn commented Jan 21, 2022

As I think that let users to execute stuff before/after provision should be something not legacy but part of the tool to adapt VVV to their usage.

Not like this, this approach is dangerous, that's why it's deprecated, and there are other ways to solve problems.

@tomjn tomjn added this to the 3.10 milestone Apr 8, 2022
@tomjn tomjn modified the milestones: 3.10, 3.11 Sep 10, 2022
@tomjn tomjn modified the milestones: 3.11, 3.13 Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants