Skip to content

Commit

Permalink
adding changes related to pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Ariel Silverman committed Aug 15, 2023
1 parent 6c2d8fe commit 6cb9ae9
Show file tree
Hide file tree
Showing 24 changed files with 237 additions and 156 deletions.
2 changes: 1 addition & 1 deletion src/Bicep.Cli.IntegrationTests/BuildCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public async Task Provider_Artifacts_Restore_From_Registry_ShouldSucceed()
File.WriteAllText(bicepFilePath, bicepFile);

// 4. create a settings object with the mock registry client and relevant features enabled
var settings = new InvocationSettings(new(TestContext, RegistryEnabled: true, ExtensibilityEnabled: true), clientFactory.Object, Repository.Create<ITemplateSpecRepositoryFactory>().Object);
var settings = new InvocationSettings(new(TestContext, RegistryEnabled: true, ExtensibilityEnabled: true, DynamicTypeLoading: true), clientFactory.Object, Repository.Create<ITemplateSpecRepositoryFactory>().Object);

// TEST
// 5. run bicep build
Expand Down
16 changes: 9 additions & 7 deletions src/Bicep.Cli.IntegrationTests/RestoreCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,10 @@ public async Task Restore_With_Force_Should_Overwrite_Existing_Cache()
param p1 string
output o1 string = p1");

var (publishOutput, publishError, publishResult) = await Bicep(settings, "publish", publishedBicepFilePath, "--target", $"br:{registry}/{repository}:v1");
var (publishOutput, publishError, exitCode) = await Bicep(settings, "publish", publishedBicepFilePath, "--target", $"br:{registry}/{repository}:v1");
using (new AssertionScope())
{
publishResult.Should().Be(0);
exitCode.Should().Be(0);
publishOutput.Should().BeEmpty();
publishError.Should().BeEmpty();
}
Expand Down Expand Up @@ -272,10 +272,10 @@ param p1 string
param p2 string
output o1 string = '${p1}${p2}'");

(publishOutput, publishError, publishResult) = await Bicep(settings, "publish", publishedBicepFilePath, "--target", $"br:{registry}/{repository}:v1", "--force");
(publishOutput, publishError, exitCode) = await Bicep(settings, "publish", publishedBicepFilePath, "--target", $"br:{registry}/{repository}:v1", "--force");
using (new AssertionScope())
{
publishResult.Should().Be(0);
exitCode.Should().Be(0);
publishOutput.Should().BeEmpty();
publishError.Should().BeEmpty();
}
Expand Down Expand Up @@ -385,13 +385,15 @@ public async Task Restore_NonExistentModules_ShouldFail(DataSet dataSet)

var settings = new InvocationSettings(new(TestContext, RegistryEnabled: dataSet.HasExternalModules), clientFactory, templateSpecRepositoryFactory);
TestContext.WriteLine($"Cache root = {settings.FeatureOverrides.CacheRootDirectory}");
var (output, error, result) = await Bicep(settings, "restore", bicepFilePath);
var (output, error, exitCode) = await Bicep(settings, "restore", bicepFilePath);

using (new AssertionScope())
{
result.Should().Be(1);
exitCode.Should().Be(1);
output.Should().BeEmpty();
error.Should().ContainAll(": Error BCP192: Unable to restore the module with reference ", "The module does not exist in the registry.");
error.Should().ContainAll(": Error BCP192: Unable to restore the module with reference ", "The artifact does not exist in the registry.");


}
}

Expand Down
8 changes: 6 additions & 2 deletions src/Bicep.Cli/Services/CompilationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Bicep.Core.Configuration;
using Bicep.Core.Diagnostics;
using Bicep.Core.Extensions;
using Bicep.Core.Features;
using Bicep.Core.FileSystem;
using Bicep.Core.Navigation;
using Bicep.Core.Registry;
Expand All @@ -29,6 +30,7 @@ public class CompilationService
private readonly IDiagnosticLogger diagnosticLogger;
private readonly IModuleDispatcher moduleDispatcher;
private readonly IConfigurationManager configurationManager;
private readonly IFeatureProviderFactory featureProviderFactory;
private readonly Workspace workspace;

public CompilationService(
Expand All @@ -37,7 +39,8 @@ public CompilationService(
BicepparamDecompiler paramDecompiler,
IDiagnosticLogger diagnosticLogger,
IModuleDispatcher moduleDispatcher,
IConfigurationManager configurationManager)
IConfigurationManager configurationManager,
IFeatureProviderFactory featureProviderFactory)
{
this.bicepCompiler = bicepCompiler;
this.decompiler = decompiler;
Expand All @@ -46,6 +49,7 @@ public CompilationService(
this.moduleDispatcher = moduleDispatcher;
this.configurationManager = configurationManager;
this.workspace = new Workspace();
this.featureProviderFactory = featureProviderFactory;
}

public async Task RestoreAsync(string inputPath, bool forceModulesRestore)
Expand All @@ -65,7 +69,7 @@ public async Task RestoreAsync(string inputPath, bool forceModulesRestore)
await moduleDispatcher.RestoreModules(modulesToRestoreReferences, forceModulesRestore);

// update the errors based on restore status
var sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(this.moduleDispatcher, this.workspace, compilation.SourceFileGrouping);
var sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(featureProviderFactory, this.moduleDispatcher, this.workspace, compilation.SourceFileGrouping);

LogDiagnostics(GetModuleRestoreDiagnosticsByBicepFile(sourceFileGrouping, originalModulesToRestore, forceModulesRestore));
}
Expand Down
10 changes: 5 additions & 5 deletions src/Bicep.Core.IntegrationTests/Emit/TemplateEmitterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ private async Task<SourceFileGrouping> GetSourceFileGrouping(DataSet dataSet)
var configManager = BicepTestConstants.CreateFilesystemConfigurationManager();
var dispatcher = new ModuleDispatcher(new DefaultModuleRegistryProvider(BicepTestConstants.EmptyServiceProvider, BicepTestConstants.FileResolver, clientFactory, templateSpecRepositoryFactory, BicepTestConstants.CreateFeatureProviderFactory(new(TestContext, RegistryEnabled: dataSet.HasExternalModules), configManager), configManager), configManager);
Workspace workspace = new();
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, workspace, PathHelper.FilePathToFileUrl(bicepFilePath));
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, workspace, PathHelper.FilePathToFileUrl(bicepFilePath), BicepTestConstants.FeatureProviderFactory);
if (await dispatcher.RestoreModules(dispatcher.GetValidModuleReferences(sourceFileGrouping.GetModulesToRestore())))
{
sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(dispatcher, workspace, sourceFileGrouping);
sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(BicepTestConstants.FeatureProviderFactory, dispatcher, workspace, sourceFileGrouping);
}

return sourceFileGrouping;
Expand Down Expand Up @@ -217,7 +217,7 @@ public void InvalidBicep_TemplateEmiterShouldNotProduceAnyTemplate(DataSet dataS

// emitting the template should fail
var dispatcher = new ModuleDispatcher(BicepTestConstants.RegistryProvider, IConfigurationManager.WithStaticConfiguration(BicepTestConstants.BuiltInConfigurationWithAllAnalyzersDisabled));
var result = this.EmitTemplate(SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, new Workspace(), PathHelper.FilePathToFileUrl(bicepFilePath)), new(), filePath);
var result = this.EmitTemplate(SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, new Workspace(), PathHelper.FilePathToFileUrl(bicepFilePath), BicepTestConstants.FeatureProviderFactory), new(), filePath);
result.Diagnostics.Should().NotBeEmpty();
result.Status.Should().Be(EmitStatus.Failed);
}
Expand All @@ -230,7 +230,7 @@ public void Valid_bicepparam_TemplateEmiter_should_produce_expected_template(Bas
var data = baselineData.GetData(TestContext);
data.Compiled.Should().NotBeNull();

var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, BicepTestConstants.ModuleDispatcher, new Workspace(), PathHelper.FilePathToFileUrl(data.Parameters.OutputFilePath));
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, BicepTestConstants.ModuleDispatcher, new Workspace(), PathHelper.FilePathToFileUrl(data.Parameters.OutputFilePath), BicepTestConstants.FeatureProviderFactory);
var result = this.EmitParam(sourceFileGrouping, data.Compiled!.OutputFilePath);

result.Diagnostics.Should().NotHaveErrors();
Expand All @@ -246,7 +246,7 @@ public void Invalid_bicepparam_TemplateEmiter_should_not_produce_a_template(Base
{
var data = baselineData.GetData(TestContext);

var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, BicepTestConstants.ModuleDispatcher, new Workspace(), PathHelper.FilePathToFileUrl(data.Parameters.OutputFilePath));
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, BicepTestConstants.ModuleDispatcher, new Workspace(), PathHelper.FilePathToFileUrl(data.Parameters.OutputFilePath), BicepTestConstants.FeatureProviderFactory);
var result = this.EmitParam(sourceFileGrouping, Path.ChangeExtension(data.Parameters.OutputFilePath, ".json"));

result.Diagnostics.Should().NotBeEmpty();
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.IntegrationTests/ExamplesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private void RunExampleTest(EmbeddedFile embeddedBicep, FeatureProviderOverrides

var configManager = IConfigurationManager.WithStaticConfiguration(BicepTestConstants.BuiltInConfigurationWithAllAnalyzersDisabled);
var dispatcher = new ModuleDispatcher(BicepTestConstants.RegistryProvider, configManager);
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, new Workspace(), PathHelper.FilePathToFileUrl(bicepFile.OutputFilePath));
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, new Workspace(), PathHelper.FilePathToFileUrl(bicepFile.OutputFilePath), BicepTestConstants.FeatureProviderFactory);
var compilation = Services.WithFeatureOverrides(features).Build().BuildCompilation(sourceFileGrouping);
var emitter = new TemplateEmitter(compilation.GetEntrypointSemanticModel());

Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.IntegrationTests/ModuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public void SourceFileGroupingBuilder_build_should_throw_diagnostic_exception_if
var mockDispatcher = Repository.Create<IModuleDispatcher>();
SetupFileReaderMock(mockFileResolver, fileUri, null, x => x.ErrorOccurredReadingFile("Mock read failure!"));

Action buildAction = () => SourceFileGroupingBuilder.Build(mockFileResolver.Object, mockDispatcher.Object, new Workspace(), fileUri);
Action buildAction = () => SourceFileGroupingBuilder.Build(mockFileResolver.Object, mockDispatcher.Object, new Workspace(), fileUri, BicepTestConstants.FeatureProviderFactory);
buildAction.Should().Throw<ErrorDiagnosticException>()
.And.Diagnostic.Should().HaveCodeAndSeverity("BCP091", DiagnosticLevel.Error).And.HaveMessage("An error occurred reading file. Mock read failure!");
}
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core.IntegrationTests/RegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ public async Task InvalidRootCachePathShouldProduceReasonableErrors()
var dispatcher = new ModuleDispatcher(new DefaultModuleRegistryProvider(EmptyServiceProvider, BicepTestConstants.FileResolver, clientFactory, templateSpecRepositoryFactory, featuresFactory, BicepTestConstants.ConfigurationManager), BicepTestConstants.ConfigurationManager);

var workspace = new Workspace();
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, workspace, fileUri);
var sourceFileGrouping = SourceFileGroupingBuilder.Build(BicepTestConstants.FileResolver, dispatcher, workspace, fileUri, featuresFactory);
if (await dispatcher.RestoreModules(dispatcher.GetValidModuleReferences(sourceFileGrouping.GetModulesToRestore())))
{
sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(dispatcher, workspace, sourceFileGrouping);
sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(featuresFactory, dispatcher, workspace, sourceFileGrouping);
}

var compilation = Services.WithFeatureOverrides(featureOverrides).Build().BuildCompilation(sourceFileGrouping);
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.UnitTests/Registry/TagEncoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void EncoderShouldThrowWhenMaxTagLengthIsExceeded() =>
[TestMethod]
public void EncodingFullyCapitalizedStringOfMaxLengthShouldNotExceedMaxLinuxFileNameLength()
{
var fullyCapitalizedTag = new string('A', IOciArtifactReference.MaxTagLength);
var fullyCapitalizedTag = new string('A', OciArtifactReferenceFacts.MaxTagLength);
var encoded = TagEncoder.Encode(fullyCapitalizedTag);

encoded.Length.Should().BeLessOrEqualTo(255);
Expand Down
1 change: 1 addition & 0 deletions src/Bicep.Core.UnitTests/ServiceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public static SourceFileGrouping BuildSourceFileGrouping(this IDependencyHelper
helper.Construct<IModuleDispatcher>(),
helper.Construct<IWorkspace>(),
entryFileUri,
helper.Construct<IFeatureProviderFactory>(),
forceModulesRestore);

public static BicepCompiler GetCompiler(this IDependencyHelper helper)
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/BicepCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public BicepCompiler(
public async Task<Compilation> CreateCompilation(Uri bicepUri, IReadOnlyWorkspace? workspace = null, bool skipRestore = false, bool forceModulesRestore = false)
{
workspace ??= new Workspace();
var sourceFileGrouping = SourceFileGroupingBuilder.Build(fileResolver, moduleDispatcher, workspace, bicepUri, forceModulesRestore);
var sourceFileGrouping = SourceFileGroupingBuilder.Build(fileResolver, moduleDispatcher, workspace, bicepUri, featureProviderFactory, forceModulesRestore);

if (!skipRestore)
{
Expand All @@ -52,7 +52,7 @@ public async Task<Compilation> CreateCompilation(Uri bicepUri, IReadOnlyWorkspac
if (await moduleDispatcher.RestoreModules(moduleDispatcher.GetValidModuleReferences(sourceFileGrouping.GetModulesToRestore())))
{
// modules had to be restored - recompile
sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(moduleDispatcher, workspace, sourceFileGrouping);
sourceFileGrouping = SourceFileGroupingBuilder.Rebuild(featureProviderFactory, moduleDispatcher, workspace, sourceFileGrouping);
}
//TODO(asilverman): I want to inject here the logic that restores the providers
}
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Modules/ModuleReferenceSchemes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public static class ModuleReferenceSchemes
{
public const string Local = "";

public const string Oci = IOciArtifactReference.Scheme;
public const string Oci = OciArtifactReferenceFacts.Scheme;

public const string TemplateSpecs = "ts";
}
Expand Down
45 changes: 44 additions & 1 deletion src/Bicep.Core/Registry/AzureContainerRegistryManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Azure;
using Azure.Containers.ContainerRegistry;
Expand Down Expand Up @@ -148,7 +149,49 @@ private static async Task<OciArtifactResult> DownloadManifestAsync(IOciArtifactR
// BUG: The SDK internally consumed the stream for validation purposes and left position at the end
stream.Position = 0;
ValidateManifestResponse(manifestResponse);
return new OciArtifactResult(client, manifestResponse.Value.Manifest, manifestResponse.Value.Digest);

var deserializedManifest = OciManifest.FromBinaryData(manifestResponse.Value.Manifest) ?? throw new InvalidOperationException("the manifest is not a valid OCI manifest");
var layers = deserializedManifest.Layers
.Select(layer => new KeyValuePair<string, BinaryData>(layer.Digest, PullLayerAsync(client, layer).Result))
.ToImmutableDictionary(pair => pair.Key, pair => pair.Value);

return new(manifestResponse.Value.Manifest, manifestResponse.Value.Digest, layers);
}

private static async Task<BinaryData> PullLayerAsync(ContainerRegistryContentClient client, OciDescriptor layer, CancellationToken cancellationToken = default)
{
Response<DownloadRegistryBlobResult> blobResult;
try
{
blobResult = await client.DownloadBlobContentAsync(layer.Digest, cancellationToken);
}
catch (RequestFailedException exception) when (exception.Status == 404)
{
throw new InvalidModuleException($"Module manifest refers to a non-existent blob with digest \"{layer.Digest}\".", exception);
}

ValidateBlobResponse(blobResult, layer);

return blobResult.Value.Content;
}

private static void ValidateBlobResponse(Response<DownloadRegistryBlobResult> blobResponse, OciDescriptor descriptor)
{
using var stream = blobResponse.Value.Content.ToStream();

if (descriptor.Size != stream.Length)
{
throw new InvalidModuleException($"Expected blob size of {descriptor.Size} bytes but received {stream.Length} bytes from the registry.");
}

stream.Position = 0;
string digestFromContents = DescriptorFactory.ComputeDigest(DescriptorFactory.AlgorithmIdentifierSha256, stream);
stream.Position = 0;

if (!string.Equals(descriptor.Digest, digestFromContents, StringComparison.Ordinal))
{
throw new InvalidModuleException($"There is a mismatch in the layer digests. Received content digest = {digestFromContents}, Requested digest = {descriptor.Digest}");
}
}

private static void ValidateManifestResponse(Response<GetManifestResult> manifestResponse)
Expand Down
22 changes: 1 addition & 21 deletions src/Bicep.Core/Registry/Oci/IOciArtifactReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,7 @@ namespace Bicep.Core.Registry.Oci
{
public interface IOciArtifactReference
{
public const string Scheme = "br";

public const int MaxRegistryLength = 255;

// must be kept in sync with the tag name regex
public const int MaxTagLength = 128;

public const int MaxRepositoryLength = 255;

// the registry component is equivalent to a host in a URI, which are case-insensitive
public static readonly IEqualityComparer<string> RegistryComparer = StringComparer.OrdinalIgnoreCase;

// repository component is case-sensitive (although regex blocks upper case)
public static readonly IEqualityComparer<string> RepositoryComparer = StringComparer.Ordinal;

// tags are case-sensitive and may contain upper and lowercase characters
public static readonly IEqualityComparer<string?> TagComparer = StringComparer.Ordinal;

// digests are case sensitive
public static readonly IEqualityComparer<string?> DigestComparer = StringComparer.Ordinal;


/// <summary>
/// Gets the registry URI.
/// </summary>
Expand Down
Loading

0 comments on commit 6cb9ae9

Please sign in to comment.