-
Notifications
You must be signed in to change notification settings - Fork 865
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
Added new agent capability AllowWorkDirectoryRepositories so repositories can be checked out below the work directory level too on self hosted agents #3475
Conversation
… in .agent config file to true or false, the default behavior is false. If AllowWorkDirectoryRepositories capability is set to true then repositories can be checked out below the work directory level too, and not just below the build directory level, by adding "../" at the beginning of the checkout path. So "../Repositories/RepoName" would then be a valid repository checkout path (but not any higher, so "../../Repositories/RepoName" is still not allowed). This allows for agent scoped repositories instead of just pipeline scoped repositories and can potentially save a lot of space on self hosted agents with multiple pipelines using the same large repository/repositories. The default behavior remains allowing only pipeline scoped repositories (below the build directory level) and it is highly recommended to not specifically allow work directory repositories for public agents as build pipelines could interfere with each other and it could pose a security risk by being able to change any files below the work directory. Also added UnitTests for this functionality.
…oot directory (build or working directory, depending on whether AllowWorkingDirectoryRepositories capability is activated) cannot be used as the checkout path for a repository, only directories below them (this is already tested in other UnitTests)
…lity_MasterConflictFix # Conflicts: # src/Agent.Plugins/RepositoryPlugin.cs # src/Microsoft.VisualStudio.Services.Agent/ConfigurationStore.cs
Fixed merge conflicts, pls maybe get to this at least when PR #3473 has been closed. It's also already lying around half a year and is a much wanted capability. Again, if it helps (with budgets or something like that), we're a Microsoft Gold Partner. If you need any information about our Gold Partnership for accounting or whatnot so you can invest a bit of time just let me know what you need. |
@echalone thanks! Let us review and test this PR |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -18,6 +18,9 @@ public class ConfigureAgent : ConfigureOrRemoveBase | |||
[Option(Constants.Agent.CommandLine.Flags.AddMachineGroupTags)] | |||
public bool AddMachineGroupTags { get; set; } | |||
|
|||
[Option(Constants.Agent.CommandLine.Flags.AllowWorkDirectoryRepositories)] | |||
public bool AllowWorkDirectoryRepositories { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to have this setting? Since to my understanding the (public) hosted agents will create a "fresh" agent each time, so there is no risk of having anyone else's code on you agent. If the hosted agents might still have code from another organisation on it this would be a very serious security issue.
Do we really need to "shared" repository to be a setting? Since a build agent can only process a single job at a time, there are no concurrency issues. When my assumption about the hosted agents having a fresh server each build there is no risk of accessing other's code. If this is the case there is an argument to be made to have repositories shared over different pipelines by default |
Could it probably make sense to save repositories in some specific folder inside of _work folder in this case (for example '_sharedRepos')? This should help to avoid conflicts between repos names and agent internals. |
In this case, it makes sense to rename this setting to make it more transparent to users. |
It's true that you'd have to be careful not to use one of the potentially used directories, though I would have left this in the responsibility of the user... if you wanna use this option you gotta know what you do so to say. |
So what's next for this? I guess we need someone to decide if we want to actually build this functionality, and if so, how it should be implemented. @AndreyIvanov42 could you weigh in here and tell us if and how we should build this? |
Right, so, in my opinion there are three options, in any case you would have to enable this option when installing the agent, so it won't be a breaking change or impact Microsoft hosted agents:
|
What if we simply go with moving the path to which to repo is checked out to a "shared" location like The hosted agents should also be fine, because a This way we wouldn't even need a setting at all |
I think it could potentially break the agents of other people who do not use a fresh agent every time and may have two repositories with the same name but which are not actually the same repos. But that's for you guys to decide if that's acceptable. If so, then yes that's another way. |
I can now additional confirm that some extensions, for example the SonarQube extension, will have problems when the sources are not checked out at the working directory of a build, which is per default either the path to the default repository or _work\1\s in a multi-checkout scenario, since SonarQube expects the sources to be below the working directory. So any change in the direction of shared repositories by default would need to consider this. This PR which would integrate a function to select a repository as the default working directory would come in handy here anyways for multi-checkouts, and it would also determine System.DefaultWorkingDirectory to be used as the variable for working directory of extensions instead of Build.SourcesDirectory (which currently always have the same values): #3479 Also, the primary repository detection, which could interfere with correct working directory detection, has an error which will be fixed with this PR: #3473 |
Changed link to issue since it was wrongfully closed by a stale-bot |
Hi @echalone thank you for your work! We have decided to implement basically what you wrote, with the difference that it is activated by an agent knob instead of a capability, as this approach seems simpler and more flexible. Here is the PR: #4423 Feel free to participate if you don't mind |
Thank you :) |
PR #4423 that has been forked from this one has been merged with master, so closing this PR now since it's done its duty |
Added new agent capability AllowWorkDirectoryRepositories. Can be activated by passing "--allowWorkDirectoryRepositories" when configuring the agent, the default behavior is for the capability to not be activated.
The feature request for this new capability can be found here: #3951
The pull request for the documentation of this new feature, which has to be approved after the feature has been implemented, can be found here: MicrosoftDocs/azure-devops-docs#11154
If the allowWorkDirectoryRepositories capability was activated then repositories can be checked out below the work directory level too, and not just below the build directory level, by adding "../" at the beginning of the checkout path. So "../Repositories/RepoName" would then be a valid repository checkout path (but not any higher, so "../../Repositories/RepoName" is still not allowed). This is to potentially save enormous amounts of agent drive space on self hosted agents with large repositories which are shared between pipelines.
The .agent configuration file for an agent with this capability activated would for example then look like this:
This setting is also visible in the agent capability page as setting "AllowWorkDirectoryRepositories".
A valid yaml checkout step checking out the repository below the working directory would for exmample then look like this (provided allowWorkDirectoryRepositories was enabled in the .agent configuration file):
This allows for agent scoped repositories instead of just pipeline scoped repositories and can potentially save a lot of space on self hosted agents with multiple pipelines using the same large repository/repositories. For example: We have one 50 GB and an additional 10 GB repository which we would have to check out and use drive space for each pipeline we use. It's just not possible to use pipelines the way they are intended then and we have to work with tricks to not fill up our drives immediatly (like triggering the same pipeline with differnt settings through other pipelines). This however would finally provide a clean way to save enormous amounts of agent drive space, and in the end also money, on self hosted agents (just imagine having to check out 60 GB for every pipeline or just once per agent). It would also allow us to finally use pipelines as intended and create one for every needed setting we have. So we really need this feature, that's why I implemented it as an option and really hope you'll incorporate into the next version(s).
The default behavior remains allowing only pipeline scoped repositories (below the build directory level) and it is highly recommended to not specifically allow work directory repositories for public agents as build pipelines could interfere with each other and it could pose a security risk by allowing users with access to yaml pipelines defintions to change any files below the work directory. I also added UnitTests for this functionality when the capability is activated and for the default behaviour (capability is not activated) which should be to continue to not allow work directory repositories.