Skip to content
This repository was archived by the owner on Nov 20, 2023. It is now read-only.

Fixed when path to Dockerfile isn't correctly expanded when service is references as repository. #1083

Open
OlegKarasik wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fixed when path to Dockerfile isn't correctly expanded when service is references as repository. #1083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c9ee88e
Fixed an issue when path to Dockerfile isn't correctly expanded when …
OlegKarasik Jun 11, 2021
459e537
Fixed spacing issues
OlegKarasik Jun 14, 2021
fac57a2
Removed trailing whitespaces as required by formatter tool
OlegKarasik Jun 15, 2021
8ae0e6f
Merge branch 'dotnet:main' into fixRepositoryWithDockerfile
OlegKarasik Mar 18, 2022
56d661b
Merge branch 'main' of https://github.com/OlegKarasik/tye into fixRep…
OlegKarasik Apr 12, 2022
3a5064a
Implemented a demo test (need to update with much simplier test proje…
OlegKarasik Apr 16, 2022
785bbfb
Updated test to use simple docker image
OlegKarasik Apr 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions src/Microsoft.Tye.Core/ApplicationFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ bool IsAzureFunctionService(ConfigService service)
}

var sw = Stopwatch.StartNew();

// This will correctly expand full paths to all service files and projects
EvaluatePaths(
configServices: services,
configRoot: config.Source.DirectoryName!,
output: output);

// Project services will be restored and evaluated before resolving all other services.
// This batching will mitigate the performance cost of running MSBuild out of process.
var projectServices = services.Where(s => !string.IsNullOrEmpty(s.Project));
Expand Down Expand Up @@ -142,6 +149,11 @@ bool IsAzureFunctionService(ConfigService service)
{
output.WriteDebugLine("Re-evaluating multi-targeted projects");

EvaluatePaths(
configServices: multiTFMProjects,
configRoot: config.Source.DirectoryName!,
output: output);

var multiTFMEvaluationResult = await EvaluateProjectsAsync(
projects: multiTFMProjects,
configRoot: config.Source.DirectoryName!,
Expand Down Expand Up @@ -259,7 +271,7 @@ bool IsAzureFunctionService(ConfigService service)
Args = configService.Args,
Build = configService.Build ?? true,
Replicas = configService.Replicas ?? 1,
DockerFile = Path.Combine(source.DirectoryName!, configService.DockerFile),
DockerFile = Path.Combine(source.DirectoryName!, configService.DockerFileFullPath!),
// Supplying an absolute path with trailing slashes fails for DockerFileContext when calling docker build, so trim trailing slash.
DockerFileContext = GetDockerFileContext(source, configService),
BuildArgs = configService.DockerFileArgs
Expand Down Expand Up @@ -503,6 +515,27 @@ service is ProjectServiceBuilder project2 &&
return root;
}

private static void EvaluatePaths(IEnumerable<ConfigService> configServices, string configRoot, OutputContext output)
{
output.WriteDebugLine("Evaluating configuration paths");

foreach (var configService in configServices)
{
if (!string.IsNullOrEmpty(configService.Project))
{
configService.ProjectFullPath = Path.Combine(
configRoot,
Environment.ExpandEnvironmentVariables(configService.Project!));
}
if (!string.IsNullOrEmpty(configService.DockerFile))
{
configService.DockerFileFullPath = Path.Combine(
configRoot,
Environment.ExpandEnvironmentVariables(configService.DockerFile!));
}
}
}

private static async Task<ProcessResult> EvaluateProjectsAsync(IEnumerable<ConfigService> projects, string configRoot, OutputContext output)
{
using var directory = TempDirectory.Create();
Expand All @@ -514,9 +547,6 @@ private static async Task<ProcessResult> EvaluateProjectsAsync(IEnumerable<Confi

foreach (var project in projects)
{
var expandedProject = Environment.ExpandEnvironmentVariables(project.Project!);
project.ProjectFullPath = Path.Combine(configRoot, expandedProject);

if (!File.Exists(project.ProjectFullPath))
{
throw new CommandException($"Failed to locate project: '{project.ProjectFullPath}'.");
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.Tye.Core/ConfigModel/ConfigService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class ConfigService
public bool External { get; set; }
public string? Image { get; set; }
public string? DockerFile { get; set; }
public string? DockerFileFullPath { get; internal set; }
public Dictionary<string, string> DockerFileArgs { get; set; } = new Dictionary<string, string>();
public string? DockerFileContext { get; set; }
public string? Project { get; set; }
Expand Down
38 changes: 38 additions & 0 deletions test/E2ETest/TyeRunTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,44 @@ public async Task MultiRepo_WorksWithCloning()
});
}

[ConditionalFact]
[SkipIfDockerNotRunning]
public async Task MultiRepo_WorksWithCloningAndDockerfile()
{
using var projectDirectory = TempDirectory.Create(preferUserDirectoryOnMacOS: true);

var content = @"
name: tye-docker-sample
services:
- name: minapp
repository: https://github.com/OlegKarasik/tye-docker-sample";
Copy link
Member

@davidfowl davidfowl Dec 12, 2022

Choose a reason for hiding this comment

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

This is problematic dependency but I'll merge this fix for now. If this repo disappears this test will start breaking @OlegKarasik

Copy link
Contributor Author

@OlegKarasik OlegKarasik Dec 12, 2022

Choose a reason for hiding this comment

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

@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.

Copy link
Member

@davidfowl davidfowl Dec 12, 2022

Choose a reason for hiding this comment

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

I wonder if it's possible to do this within the tye repo itself?

Copy link
Contributor Author

@OlegKarasik OlegKarasik Jul 8, 2023

Choose a reason for hiding this comment

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

@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!);
}


var yamlFile = Path.Combine(projectDirectory.DirectoryPath, "tye.yaml");
var projectFile = new FileInfo(yamlFile);
await File.WriteAllTextAsync(yamlFile, content);

// Debug targets can be null if not specified, so make sure calling host.Start does not throw.
var outputContext = new OutputContext(_sink, Verbosity.Debug);
var application = await ApplicationFactory.CreateAsync(outputContext, projectFile);

var handler = new HttpClientHandler
{
ServerCertificateCustomValidationCallback = (a, b, c, d) => true,
AllowAutoRedirect = false
};

var client = new HttpClient(new RetryHandler(handler));

await RunHostingApplication(application, new HostOptions(), async (app, uri) =>
{
var appUri = await GetServiceUrl(client, uri, "minapp");

var appResponse = await client.GetAsync(appUri);

Assert.True(appResponse.IsSuccessStatusCode);
});
}

[ConditionalFact]
[SkipIfDockerNotRunning]
public async Task DockerFileTest()
Expand Down