Skip to content

Commit

Permalink
feat: add custom dns zone name in networkSpec
Browse files Browse the repository at this point in the history
fix: add conversion for PrivateDNSZoneName && removed it from v1alpha3

fix: enforce immutability on PrivateDNSZoneName
Signed-off-by: Cyril Corbon <corboncyril@gmail.com>
  • Loading branch information
cyril-corbon committed Apr 8, 2021
1 parent d46f105 commit a6ff323
Show file tree
Hide file tree
Showing 20 changed files with 301 additions and 1 deletion.
11 changes: 11 additions & 0 deletions api/v1alpha3/azurecluster_conversion.go
Expand Up @@ -24,6 +24,7 @@ import (
v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
apiv1alpha3 "sigs.k8s.io/cluster-api/api/v1alpha3"
apiv1alpha4 "sigs.k8s.io/cluster-api/api/v1alpha4"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
)

Expand All @@ -45,6 +46,12 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
dst.Annotations = nil
}
}
restored := &infrav1alpha4.AzureCluster{}
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
return err
}

dst.Spec.NetworkSpec.PrivateDNSZoneName = restored.Spec.NetworkSpec.PrivateDNSZoneName

return nil
}
Expand All @@ -63,6 +70,10 @@ func (dst *AzureCluster) ConvertFrom(srcRaw conversion.Hub) error { // nolint
}
dst.Annotations[azureEnvironmentAnnotation] = src.Spec.AzureEnvironment
}
// Preserve Hub data on down-conversion.
if err := utilconversion.MarshalData(src, dst); err != nil {
return err
}

return nil
}
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions api/v1alpha4/azurecluster_validation.go
Expand Up @@ -21,6 +21,7 @@ import (
"net"
"regexp"

valid "github.com/asaskevich/govalidator"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down Expand Up @@ -97,6 +98,7 @@ func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel
}
allErrs = append(allErrs, validateSubnets(networkSpec.Subnets, fldPath.Child("subnets"))...)
}

var cidrBlocks []string
subnet, err := networkSpec.GetControlPlaneSubnet()
if err != nil {
Expand All @@ -105,6 +107,8 @@ func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel
cidrBlocks = subnet.CIDRBlocks

allErrs = append(allErrs, validateAPIServerLB(networkSpec.APIServerLB, old.APIServerLB, cidrBlocks, fldPath.Child("apiServerLB"))...)
allErrs = append(allErrs, validatePrivateDNSZoneName(networkSpec, fldPath)...)

if len(allErrs) == 0 {
return nil
}
Expand Down Expand Up @@ -264,3 +268,27 @@ func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []stri

return allErrs
}

// validatePrivateDNSZoneName validate the PrivateDNSZoneName
func validatePrivateDNSZoneName(networkSpec NetworkSpec, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
apiLBPath := field.NewPath("spec").Child("networkSpec").Child("apiServerLB").Child("type")

if len(networkSpec.PrivateDNSZoneName) > 0 {
if networkSpec.APIServerLB.Type == "" || string(networkSpec.APIServerLB.Type) != "Internal" {
allErrs = append(allErrs, field.Invalid(apiLBPath, networkSpec.APIServerLB.Type,
"PrivateDNSZoneName is available only if APIServerLB.Type is Internal"))
}
if !valid.IsDNSName(networkSpec.PrivateDNSZoneName) {
allErrs = append(allErrs, field.Invalid(fldPath, networkSpec.PrivateDNSZoneName,
"Private DNS Zone Name can only contains alphanumeric characters, underscores and dashes, must end with an alphanumeric character",
))
}

}
if len(allErrs) == 0 {
return nil
}

return allErrs
}
92 changes: 92 additions & 0 deletions api/v1alpha4/azurecluster_validation_test.go
Expand Up @@ -757,6 +757,84 @@ func TestValidateAPIServerLB(t *testing.T) {
})
}
}
func TestPrivateDNSZoneName(t *testing.T) {
g := NewWithT(t)

testcases := []struct {
name string
network NetworkSpec
wantErr bool
expectedErr field.Error
}{
{
name: "testInvalidPrivateDNSZoneName",
network: NetworkSpec{
PrivateDNSZoneName: "wrong@d_ns.io",
APIServerLB: createValidAPIServerIntenralLB(),
},
expectedErr: field.Error{
Type: "FieldValueInvalid",
Field: "spec.networkSpec.privateDNSZoneName",
BadValue: "wrong@d_ns.io",
Detail: "Private DNS Zone Name can only contains alphanumeric characters, underscores and dashes, must end with an alphanumeric character",
},
wantErr: true,
},
{
name: "testValidPrivateDNSZoneName",
network: NetworkSpec{
PrivateDNSZoneName: "good.dns.io",
APIServerLB: createValidAPIServerIntenralLB(),
},
wantErr: false,
},
{
name: "testValidPrivateDNSZoneNameWithUnderscore",
network: NetworkSpec{
PrivateDNSZoneName: "_good.__dns.io",
APIServerLB: createValidAPIServerIntenralLB(),
},
wantErr: false,
},
{
name: "testBadAPIServerLBType",
network: NetworkSpec{
PrivateDNSZoneName: "good.dns.io",
APIServerLB: LoadBalancerSpec{
Name: "my-lb",
Type: Public,
},
},
expectedErr: field.Error{
Type: "FieldValueInvalid",
Field: "spec.networkSpec.apiServerLB.type",
BadValue: "Public",
Detail: "PrivateDNSZoneName is available only if APIServerLB.Type is Internal",
},
wantErr: true,
},
}

for _, test := range testcases {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()
err := validatePrivateDNSZoneName(test.network, field.NewPath("spec", "networkSpec", "privateDNSZoneName"))
if test.wantErr {
g.Expect(err).NotTo(HaveLen(0))
found := false
for _, actual := range err {
if actual.Error() == test.expectedErr.Error() {
found = true
}
}
g.Expect(found).To(BeTrue())
} else {
g.Expect(err).To(HaveLen(0))
}
})
}
}

func createValidCluster() *AzureCluster {
return &AzureCluster{
Expand Down Expand Up @@ -809,3 +887,17 @@ func createValidAPIServerLB() LoadBalancerSpec {
Type: Public,
}
}

func createValidAPIServerIntenralLB() LoadBalancerSpec {
return LoadBalancerSpec{
Name: "my-lb",
SKU: SKUStandard,
FrontendIPs: []FrontendIP{
{
Name: "ip-config-private",
PrivateIPAddress: "10.10.1.1",
},
},
Type: Internal,
}
}
7 changes: 7 additions & 0 deletions api/v1alpha4/azurecluster_webhook.go
Expand Up @@ -91,6 +91,13 @@ func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
)
}

if !reflect.DeepEqual(c.Spec.NetworkSpec.PrivateDNSZoneName, old.Spec.NetworkSpec.PrivateDNSZoneName) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "NetworkSpec", "PrivateDNSZoneName"),
c.Spec.NetworkSpec.PrivateDNSZoneName, "field is immutable"),
)
}

if len(allErrs) == 0 {
return c.validateCluster(old)
}
Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha4/types.go
Expand Up @@ -58,6 +58,10 @@ type NetworkSpec struct {
// APIServerLB is the configuration for the control-plane load balancer.
// +optional
APIServerLB LoadBalancerSpec `json:"apiServerLB,omitempty"`

// PrivateDNSZoneName defines the zone name for the Azure Private DNS.
// +optional
PrivateDNSZoneName string `json:"privateDNSZoneName,omitempty"`
}

// VnetSpec configures an Azure virtual network.
Expand Down
1 change: 1 addition & 0 deletions azure/interfaces.go
Expand Up @@ -70,6 +70,7 @@ type NetworkDescriber interface {
APIServerLBName() string
APIServerLBPoolName(string) string
IsAPIServerPrivate() bool
GetPrivateDNSZoneName() string
OutboundLBName(string) string
OutboundPoolName(string) string
}
Expand Down
28 changes: 28 additions & 0 deletions azure/mocks/service_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion azure/scope/cluster.go
Expand Up @@ -254,7 +254,7 @@ func (s *ClusterScope) PrivateDNSSpec() *azure.PrivateDNSSpec {
var spec *azure.PrivateDNSSpec
if s.IsAPIServerPrivate() {
spec = &azure.PrivateDNSSpec{
ZoneName: azure.GeneratePrivateDNSZoneName(s.ClusterName()),
ZoneName: s.GetPrivateDNSZoneName(),
VNetName: s.Vnet().Name,
VNetResourceGroup: s.Vnet().ResourceGroup,
LinkName: azure.GenerateVNetLinkName(s.Vnet().Name),
Expand Down Expand Up @@ -343,6 +343,14 @@ func (s *ClusterScope) APIServerPrivateIP() string {
return s.APIServerLB().FrontendIPs[0].PrivateIPAddress
}

// GetPrivateDNSZoneName returns the Private DNS Zone from the spec or generate it from cluster name.
func (s *ClusterScope) GetPrivateDNSZoneName() string {
if len(s.AzureCluster.Spec.NetworkSpec.PrivateDNSZoneName) > 0 {
return s.AzureCluster.Spec.NetworkSpec.PrivateDNSZoneName
}
return azure.GeneratePrivateDNSZoneName(s.ClusterName())
}

// APIServerLBPoolName returns the API Server LB backend pool name.
func (s *ClusterScope) APIServerLBPoolName(loadBalancerName string) string {
return azure.GenerateBackendAddressPoolName(loadBalancerName)
Expand Down
6 changes: 6 additions & 0 deletions azure/scope/managedcontrolplane.go
Expand Up @@ -240,3 +240,9 @@ func (s *ManagedControlPlaneScope) OutboundLBName(_ string) string {
func (s *ManagedControlPlaneScope) OutboundPoolName(_ string) string {
return "aksOutboundBackendPool" // hard-coded in aks
}

// GetPrivateDNSZoneName returns the Private DNS Zone from the spec or generate it from cluster name
// Currently always empty as managed control planes do not currently implement private clusters.
func (s *ManagedControlPlaneScope) GetPrivateDNSZoneName() string {
return ""
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions azure/services/routetables/mock_routetables/routetables_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a6ff323

Please sign in to comment.