Fixed when path to Dockerfile isn't correctly expanded when service is references as repository. #1083
Conversation
…referencing service as repository.
OlegKarasik
requested review from
davidfowl,
jkotalik,
JunTaoLuo and
rynowak
as code owners
|
Any way to test this? |
Yes. You can checkout this repository (I used it in one of my presentations, so please ignore the things written in You can at first run it with normal |
|
I meant an automated test as part of this PR |
No, currently there is no such test included in a PR but I can take a look on how to implement in. |
|
I see the automated tests in the repository (there were none at the time of PR). I will dig into how to create one for my case. |
|
@davidfowl I have implemented the automated test |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| name: tye-docker-sample | ||
| services: | ||
| - name: minapp | ||
| repository: https://github.com/OlegKarasik/tye-docker-sample"; |
There was a problem hiding this comment.
This is problematic dependency but I'll merge this fix for now. If this repo disappears this test will start breaking @OlegKarasik
There was a problem hiding this comment.
@davidfowl, I understand the issue with the repo. I personally have no plans to remove it but in any case, may be there is a way to fork it into more persistent (and maintainable by the team) place? I am good with this.
There was a problem hiding this comment.
I wonder if it's possible to do this within the tye repo itself?
There was a problem hiding this comment.
@davidfowl, I don't think this is possible without introducing changes to how tye searches for configuration files. Currently this is done in the ConfigFileFinder class, which does search in the root of the passed directory:
public static bool TryFindSupportedFile(
string directoryPath,
[NotNullWhen(true)] out string? filePath,
[MaybeNullWhen(true)] out string? errorMessage,
string[]? fileFormats = null)
{
fileFormats ??= FileFormats;
foreach (var format in fileFormats)
{
var files = Directory.GetFiles(directoryPath, format);
switch (files.Length)
{
case 1:
errorMessage = null;
filePath = files[0];
return true;
case 0:
continue;
}
errorMessage = $"More than one matching file was found in directory '{directoryPath}'.";
filePath = default;
return false;
}
errorMessage = $"No project project file or solution was found in directory '{directoryPath}'.";
filePath = default;
return false;
}This method is used to find a configuration file in the directory of cloned repository inside ApplicationFactory class:
var clonePath = Path.Combine(rootConfig.Source.DirectoryName!, path, configService.Name);
if (!Directory.Exists(clonePath))
{
if (!await GitDetector.Instance.IsGitInstalled.Value)
{
throw new CommandException($"Cannot clone repository {configService.Repository} because git is not installed. Please install git if you'd like to use \"repository\" in tye.yaml.");
}
var result = await ProcessUtil.RunAsync("git", $"clone {configService.Repository} \"{clonePath}\"", workingDirectory: rootConfig.Source.DirectoryName, throwOnError: false);
if (result.ExitCode != 0)
{
throw new CommandException($"Failed to clone repository {configService.Repository} with exit code {result.ExitCode}.{Environment.NewLine}{result.StandardError}{result.StandardOutput}.");
}
}
if (!ConfigFileFinder.TryFindSupportedFile(clonePath, out var file, out var errorMessage))
{
throw new CommandException(errorMessage!);
}
|
Good morning, Are there any changes left to be done before the PR can be merged? |
Hello,
While experimenting with
tyeI have found a bug related to correct path of Dockerfile based services referenced as repository. Here is how I reproduced the issue:remote. In this repository create an API serviceremote-api, create a Dockerfile for it and create the followingtye.yaml:If you run any
tyecommand (build,run, etc) it will work fine. So the configuration is correct. Now create a one more repositorysource. In this repository create an API servicesource-apiand create the followingtye.yaml:Now if you run
tye buildyou will receive the following error:Running
tye build -v Debugreveals the reason:However, the path should be
D:\Projects\Bug\.tye\deps\remote\remote-apibecauseremoterepository is downloaded into deps directory.To fix this issue I have separated the logic of
FullPathresolutions (previously it was done only for.csprojfiles as part of project evaluation) into separate method. Added aDockerFileFullPathproperty toConfigServiceand implemented the same resolution for it as it was previously done forProjectFileFullPath.Hope this PR will be useful.