-
Notifications
You must be signed in to change notification settings - Fork 111
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
Improved namespace handling for manifest reporting #1423
Open
APErebus
wants to merge
4
commits into
main
Choose a base branch
from
ap/improved-namespacing-for-manifests
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
49 changes: 49 additions & 0 deletions
49
source/Calamari.Tests/KubernetesFixtures/ApiResourceOutputParserTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
using System.Collections.Generic; | ||
using Calamari.Kubernetes; | ||
using FluentAssertions; | ||
using NUnit.Framework; | ||
|
||
namespace Calamari.Tests.KubernetesFixtures | ||
{ | ||
[TestFixture] | ||
public class ApiResourceOutputParserTests | ||
{ | ||
[Test] | ||
public void ShouldParseCorrectly() | ||
{ | ||
List<string> outputLines = new List<string> | ||
{ | ||
"NAME SHORTNAMES APIVERSION NAMESPACED KIND VERBS CATEGORIES", | ||
"bindings v1 true Binding create ", | ||
"componentstatuses cs v1 false ComponentStatus get,list ", | ||
"configmaps cm v1 true ConfigMap create,delete,deletecollection,get,list,patch,update,watch ", | ||
"endpoints ep v1 true Endpoints create,delete,deletecollection,get,list,patch,update,watch ", | ||
"events ev v1 true Event create,delete,deletecollection,get,list,patch,update,watch ", | ||
"limitranges limits v1 true LimitRange create,delete,deletecollection,get,list,patch,update,watch ", | ||
"namespaces ns v1 false Namespace create,delete,get,list,patch,update,watch ", | ||
"nodes no v1 false Node create,delete,deletecollection,get,list,patch,update,watch ", | ||
"persistentvolumeclaims pvc v1 true PersistentVolumeClaim create,delete,deletecollection,get,list,patch,update,watch ", | ||
"persistentvolumes pv v1 false PersistentVolume create,delete,deletecollection,get,list,patch,update,watch ", | ||
"pods po v1 true Pod create,delete,deletecollection,get,list,patch,update,watch all", | ||
"podtemplates v1 true PodTemplate create,delete,deletecollection,get,list,patch,update,watch ", | ||
"replicationcontrollers rc v1 true ReplicationController create,delete,deletecollection,get,list,patch,update,watch all", | ||
"resourcequotas quota v1 true ResourceQuota create,delete,deletecollection,get,list,patch,update,watch ", | ||
"secrets v1 true Secret create,delete,deletecollection,get,list,patch,update,watch ", | ||
"serviceaccounts sa v1 true ServiceAccount create,delete,deletecollection,get,list,patch,update,watch ", | ||
"services svc v1 true Service create,delete,deletecollection,get,list,patch,update,watch all", | ||
"mutatingwebhookconfigurations admissionregistration.k8s.io/v1 false MutatingWebhookConfiguration create,delete,deletecollection,get,list,patch,update,watch api-extensions", | ||
"validatingadmissionpolicies admissionregistration.k8s.io/v1 false ValidatingAdmissionPolicy create,delete,deletecollection,get,list,patch,update,watch api-extensions", | ||
"validatingadmissionpolicybindings admissionregistration.k8s.io/v1 false ValidatingAdmissionPolicyBinding create,delete,deletecollection,get,list,patch,update,watch api-extensions", | ||
"validatingwebhookconfigurations admissionregistration.k8s.io/v1 false ValidatingWebhookConfiguration create,delete,deletecollection,get,list,patch,update,watch api-extensions", | ||
"customresourcedefinitions crd,crds apiextensions.k8s.io/v1 false CustomResourceDefinition create,delete,deletecollection,get,list,patch,update,watch api-extensions", | ||
"apiservices apiregistration.k8s.io/v1 false APIService create,delete,deletecollection,get,list,patch,update,watch api-extensions", | ||
"controllerrevisions apps/v1 true ControllerRevision create,delete,deletecollection,get,list,patch,update,watch " | ||
}; | ||
|
||
var parsedResult = ApiResourceOutputParser.ParseKubectlApiResourceOutput(outputLines); | ||
|
||
parsedResult.Should() | ||
.NotBeEmpty(); | ||
} | ||
} | ||
} |
61 changes: 61 additions & 0 deletions
61
source/Calamari.Tests/KubernetesFixtures/Builders/ApiResourcesScopeLookupBuilder.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
using System.Collections.Generic; | ||
using Calamari.Kubernetes; | ||
using Octopus.CoreUtilities.Extensions; | ||
|
||
namespace Calamari.Tests.KubernetesFixtures.Builders | ||
{ | ||
public class ApiResourcesScopeLookupBuilder | ||
{ | ||
Dictionary<ApiResourceIdentifier, bool> namespacedResourcesLookup = new Dictionary<ApiResourceIdentifier, bool>(); | ||
|
||
public ApiResourcesScopeLookupBuilder WithNonNamespacedApiResource(ApiResourceIdentifier apiResourceIdentifier) | ||
{ | ||
namespacedResourcesLookup.Add(apiResourceIdentifier, false); | ||
|
||
return this; | ||
} | ||
|
||
public ApiResourcesScopeLookupBuilder WithNamespacedApiResource(ApiResourceIdentifier apiResourceIdentifier) | ||
{ | ||
namespacedResourcesLookup.Add(apiResourceIdentifier, true); | ||
|
||
return this; | ||
} | ||
|
||
public ApiResourcesScopeLookupBuilder WithoutApiResource(ApiResourceIdentifier apiResourceIdentifier) | ||
{ | ||
if (namespacedResourcesLookup.ContainsKey(apiResourceIdentifier)) | ||
{ | ||
namespacedResourcesLookup.Remove(apiResourceIdentifier); | ||
} | ||
|
||
return this; | ||
} | ||
|
||
public ApiResourcesScopeLookupBuilder WithDefaults() | ||
{ | ||
namespacedResourcesLookup.AddRangeUnique(ApiResourceScopeLookup.DefaultResourceScopeLookup); | ||
return this; | ||
} | ||
|
||
public IApiResourceScopeLookup Build() | ||
{ | ||
return new StubApiResourceScopeLookup(namespacedResourcesLookup); | ||
} | ||
|
||
class StubApiResourceScopeLookup : IApiResourceScopeLookup | ||
{ | ||
readonly Dictionary<ApiResourceIdentifier, bool> lookup; | ||
|
||
public StubApiResourceScopeLookup(Dictionary<ApiResourceIdentifier, bool> lookup) | ||
{ | ||
this.lookup = lookup; | ||
} | ||
|
||
public bool TryGetIsNamespaceScoped(ApiResourceIdentifier apiResourceIdentifier, out bool isNamespaceScoped) | ||
{ | ||
return lookup.TryGetValue(apiResourceIdentifier, out isNamespaceScoped); | ||
} | ||
} | ||
} | ||
} |
160 changes: 160 additions & 0 deletions
160
source/Calamari.Tests/KubernetesFixtures/KubernetesManifestNamespaceResolverTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
using System.IO; | ||
using System.Linq; | ||
using Calamari.Common.Plumbing.Variables; | ||
using Calamari.Kubernetes; | ||
using Calamari.Testing.Helpers; | ||
using FluentAssertions; | ||
using NSubstitute; | ||
using NUnit.Framework; | ||
using YamlDotNet.RepresentationModel; | ||
|
||
namespace Calamari.Tests.KubernetesFixtures | ||
{ | ||
[TestFixture] | ||
public class KubernetesManifestNamespaceResolverTests | ||
{ | ||
KubernetesManifestNamespaceResolver sut; | ||
IApiResourceScopeLookup apiResourceScopeLookup; | ||
|
||
[SetUp] | ||
public void SetUp() | ||
{ | ||
apiResourceScopeLookup = Substitute.For<IApiResourceScopeLookup>(); | ||
var memoryLog = new InMemoryLog(); | ||
sut = new KubernetesManifestNamespaceResolver(apiResourceScopeLookup, memoryLog); | ||
} | ||
|
||
[TestCaseSource(nameof(ResolveNamespaceTestData))] | ||
public void ResolveNamespace_ShouldResolveExpectedNamespace(string yaml, bool isKnownResourceType, bool isNamespacedResourceType, CalamariVariables variables, string expectedNamespace) | ||
{ | ||
// Arrange | ||
apiResourceScopeLookup.TryGetIsNamespaceScoped(Arg.Any<ApiResourceIdentifier>(), out var outParam) | ||
.Returns(ci => | ||
{ | ||
ci[1] = isNamespacedResourceType; | ||
return isKnownResourceType; | ||
}); | ||
|
||
var yamlStream = new YamlStream(); | ||
yamlStream.Load(new StringReader(yaml)); | ||
var rootNode = (YamlMappingNode)yamlStream.Documents.First().RootNode; | ||
|
||
|
||
// Act | ||
var @namespace = sut.ResolveNamespace(rootNode, variables ?? new CalamariVariables()); | ||
|
||
// Assert | ||
@namespace.Should().Be(expectedNamespace); | ||
} | ||
|
||
public static object[] ResolveNamespaceTestData = { | ||
//Namespaced resources | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource | ||
namespace: ABC", | ||
true, | ||
true, | ||
null, | ||
"ABC" | ||
).SetName("Known namespaced resource + namespace in manifest = Manifest namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
true, | ||
true, | ||
new CalamariVariables | ||
{ | ||
{SpecialVariables.Helm.Namespace, "DEF"} | ||
}, | ||
"DEF" | ||
).SetName("Known namespaced resource + namespace not in manifest + helm namespace variable = helm namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
true, | ||
true, | ||
new CalamariVariables | ||
{ | ||
{SpecialVariables.Namespace, "GHI"} | ||
}, | ||
"GHI" | ||
).SetName("Known namespaced resource + namespace not in manifest + namespace variable = variable namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
true, | ||
true, | ||
null, | ||
"default" | ||
).SetName("Known namespaced resource + namespace not in manifest + no namespace variable = default namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource | ||
namespace: ABC123", | ||
false, //unknown resource | ||
true, | ||
null, | ||
"ABC123" | ||
).SetName("Unknown namespaced resource + namespace in manifest = manifest namespace used"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
false, //unknown resource | ||
true, | ||
new CalamariVariables | ||
{ | ||
{ SpecialVariables.Namespace, "GHI123" } | ||
}, | ||
"GHI123" | ||
).SetName("Unknown namespaced resource + namespace not in manifest + namespace variable = variable namespace"), | ||
//Non-namespaced resources | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource | ||
namespace: ABC", | ||
true, | ||
false, //non-namespaced | ||
null, | ||
null | ||
).SetName("Known non-namespaced resource + namespace in manifest = null namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
true, | ||
false, //non-namespaced | ||
null, | ||
null | ||
).SetName("Known non-namespaced resource + namespace not in manifest = null namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource | ||
namespace: ABC123", | ||
false, //unknown resource | ||
false, //non-namespaced | ||
null, | ||
"ABC123" | ||
).SetName("Unknown non-namespaced resource + namespace in manifest = manifest namespace used"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
false, //unknown resource | ||
false, //non-namespaced | ||
null, | ||
"default" | ||
).SetName("Unknown non-namespaced resource + namespace not in manifest = default namespace"), | ||
new TestCaseData(@" | ||
metadata: | ||
name: resource", | ||
false, //unknown resource | ||
false, //non-namespaced | ||
new CalamariVariables | ||
{ | ||
{ SpecialVariables.Namespace, "GHI123" } | ||
}, | ||
"GHI123" | ||
).SetName("Unknown non-namespaced resource + namespace not in manifest + namespace variable = variable namespace") | ||
|
||
}; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Would be worth asserting that the output is as expected - I'd be happy with a really cut down list compared to what you've got to save the typing.
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.
ah yeah, I meant to do that. I think the original reason it was like this was I was using the test to debug and fix it... Good catch!