From a46cd1a58740c05fd0636afdcc5a6bdb34b46841 Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Wed, 22 Apr 2020 14:59:50 -0700 Subject: [PATCH 01/11] Move go generate to the end of the generate make target --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e613cb07c46..8551bf79bec 100644 --- a/Makefile +++ b/Makefile @@ -172,7 +172,6 @@ generate: ## Generate code .PHONY: generate-go generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related generate targets - go generate ./... $(CONTROLLER_GEN) \ paths=./api/... \ object:headerFile=./hack/boilerplate/boilerplate.generatego.txt @@ -181,6 +180,7 @@ generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related g --input-dirs=./api/v1alpha2 \ --output-file-base=zz_generated.conversion \ --go-header-file=./hack/boilerplate/boilerplate.generatego.txt + go generate ./... .PHONY: generate-manifests generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. From 9fb23570d699d2fd5af8ccbe3a9bb320be95f4ec Mon Sep 17 00:00:00 2001 From: Ace Eldeib Date: Thu, 23 Apr 2020 10:39:40 -0700 Subject: [PATCH 02/11] :sparkles: add exp folder to prepare for aks/machinepool --- .dockerignore | 1 + Makefile | 4 +++ exp/PROJECT | 4 +++ exp/README.md | 9 +++++++ exp/api/v1alpha3/groupversion_info.go | 36 +++++++++++++++++++++++++++ exp/controllers/.gitkeep | 0 exp/doc.go | 17 +++++++++++++ 7 files changed, 71 insertions(+) create mode 100644 exp/PROJECT create mode 100644 exp/README.md create mode 100644 exp/api/v1alpha3/groupversion_info.go create mode 100644 exp/controllers/.gitkeep create mode 100644 exp/doc.go diff --git a/.dockerignore b/.dockerignore index 6989619f7cc..db1740e41b3 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,6 +5,7 @@ !/api/** !/cloud/** !/controllers/** +!/exp/** !/pkg/** !/main.go !/go.mod diff --git a/Makefile b/Makefile index 8551bf79bec..56ea20dc5fe 100644 --- a/Makefile +++ b/Makefile @@ -50,6 +50,7 @@ KUBECTL=$(TOOLS_BIN_DIR)/kubectl KUSTOMIZE := $(abspath $(TOOLS_BIN_DIR)/kustomize) MOCKGEN := $(TOOLS_BIN_DIR)/mockgen RELEASE_NOTES := $(TOOLS_DIR)/$(RELEASE_NOTES_BIN) +EXP_DIR := exp # Define Docker related variables. Releases should modify and double check these vars. REGISTRY ?= gcr.io/$(shell gcloud config get-value project) @@ -174,6 +175,7 @@ generate: ## Generate code generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related generate targets $(CONTROLLER_GEN) \ paths=./api/... \ + paths=./$(EXP_DIR)/api/... \ object:headerFile=./hack/boilerplate/boilerplate.generatego.txt $(CONVERSION_GEN) \ @@ -186,12 +188,14 @@ generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related g generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. $(CONTROLLER_GEN) \ paths=./api/... \ + paths=./$(EXP_DIR)/api/... \ crd:crdVersions=v1 \ output:crd:dir=$(CRD_ROOT) \ output:webhook:dir=$(WEBHOOK_ROOT) \ webhook $(CONTROLLER_GEN) \ paths=./controllers/... \ + paths=./$(EXP_DIR)/controllers/... \ output:rbac:dir=$(RBAC_ROOT) \ rbac:roleName=manager-role diff --git a/exp/PROJECT b/exp/PROJECT new file mode 100644 index 00000000000..c5eda7e4dc7 --- /dev/null +++ b/exp/PROJECT @@ -0,0 +1,4 @@ +domain: exp.infrastructure.x-k8s.io +repo: sigs.k8s.io/cluster-api-provider-azure/exp +version: "2" +resources: [] diff --git a/exp/README.md b/exp/README.md new file mode 100644 index 00000000000..9faa282b203 --- /dev/null +++ b/exp/README.md @@ -0,0 +1,9 @@ +# exp + +This subrepository holds experimental code and API types. + +**Warning**: Packages here are experimental and unreliable. Some may one day be promoted to the main repository, or they may be modified arbitrarily or even disappear altogether. + +In short, code in this subrepository is not subject to any compatibility or deprecation promise. + +For policy around graduation timeline, see [Cluster API Exp](https://github.com/kubernetes-sigs/cluster-api/tree/master/exp). diff --git a/exp/api/v1alpha3/groupversion_info.go b/exp/api/v1alpha3/groupversion_info.go new file mode 100644 index 00000000000..897b515a0ce --- /dev/null +++ b/exp/api/v1alpha3/groupversion_info.go @@ -0,0 +1,36 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package v1alpha3 contains API Schema definitions for the exp v1alpha3 API group +// +kubebuilder:object:generate=true +// +groupName=exp.infrastructure.cluster.x-k8s.io +package v1alpha3 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "exp.infrastructure.cluster.x-k8s.io", Version: "v1alpha3"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) diff --git a/exp/controllers/.gitkeep b/exp/controllers/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/exp/doc.go b/exp/doc.go new file mode 100644 index 00000000000..8a32e6835ab --- /dev/null +++ b/exp/doc.go @@ -0,0 +1,17 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package exp From 9734e9ff7e546b16f0a8d961cd13d12a068a5beb Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Mon, 20 Apr 2020 12:52:32 -0700 Subject: [PATCH 03/11] :runner: Remove all TODOs in the codebase --- Makefile | 1 - README.md | 8 +-- api/v1alpha2/zz_generated.conversion.go | 30 ---------- api/v1alpha3/types.go | 59 ------------------- api/v1alpha3/zz_generated.deepcopy.go | 20 ------- cloud/services/subnets/subnets.go | 1 - .../virtualmachineextensions.go | 6 -- .../virtualmachines/virtualmachines.go | 1 - .../virtualnetworks/virtualnetworks.go | 1 - controllers/azurecluster_controller.go | 2 - controllers/azuremachine_controller.go | 23 ++------ controllers/azuremachine_reconciler.go | 1 - hack/tools/go.mod | 14 ----- hack/tools/go.sum | 4 +- scripts/ci-conformance.sh | 1 - test/e2e/generators/capi.go | 1 - test/e2e/generators/capz.go | 1 - test/integration/metacluster_test.go | 3 - 18 files changed, 8 insertions(+), 169 deletions(-) diff --git a/Makefile b/Makefile index 56ea20dc5fe..4c93d3dcf03 100644 --- a/Makefile +++ b/Makefile @@ -177,7 +177,6 @@ generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related g paths=./api/... \ paths=./$(EXP_DIR)/api/... \ object:headerFile=./hack/boilerplate/boilerplate.generatego.txt - $(CONVERSION_GEN) \ --input-dirs=./api/v1alpha2 \ --output-file-base=zz_generated.conversion \ diff --git a/README.md b/README.md index b6844882c27..f69d461023a 100644 --- a/README.md +++ b/README.md @@ -19,11 +19,7 @@ hybrid deployments of Kubernetes. Check out the [Cluster API Quick Start][quickstart] to create your first Kubernetes cluster on Azure using Cluster API. -## Features - -TODO - ---- +------ ## Support Policy @@ -47,7 +43,7 @@ Each version of Cluster API for Azure will attempt to support at least two Kuber **NOTE:** As the versioning for this project is tied to the versioning of Cluster API, future modifications to this policy may be made to more closely align with other providers in the Cluster API ecosystem. ---- +------ ## Documentation diff --git a/api/v1alpha2/zz_generated.conversion.go b/api/v1alpha2/zz_generated.conversion.go index 7c964c737df..ba42f446c33 100644 --- a/api/v1alpha2/zz_generated.conversion.go +++ b/api/v1alpha2/zz_generated.conversion.go @@ -137,16 +137,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*AzureResourceReference)(nil), (*v1alpha3.AzureResourceReference)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha2_AzureResourceReference_To_v1alpha3_AzureResourceReference(a.(*AzureResourceReference), b.(*v1alpha3.AzureResourceReference), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha3.AzureResourceReference)(nil), (*AzureResourceReference)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha3_AzureResourceReference_To_v1alpha2_AzureResourceReference(a.(*v1alpha3.AzureResourceReference), b.(*AzureResourceReference), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*BackendPool)(nil), (*v1alpha3.BackendPool)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha2_BackendPool_To_v1alpha3_BackendPool(a.(*BackendPool), b.(*v1alpha3.BackendPool), scope) }); err != nil { @@ -788,26 +778,6 @@ func Convert_v1alpha3_AzureMachineTemplateSpec_To_v1alpha2_AzureMachineTemplateS return autoConvert_v1alpha3_AzureMachineTemplateSpec_To_v1alpha2_AzureMachineTemplateSpec(in, out, s) } -func autoConvert_v1alpha2_AzureResourceReference_To_v1alpha3_AzureResourceReference(in *AzureResourceReference, out *v1alpha3.AzureResourceReference, s conversion.Scope) error { - out.ID = (*string)(unsafe.Pointer(in.ID)) - return nil -} - -// Convert_v1alpha2_AzureResourceReference_To_v1alpha3_AzureResourceReference is an autogenerated conversion function. -func Convert_v1alpha2_AzureResourceReference_To_v1alpha3_AzureResourceReference(in *AzureResourceReference, out *v1alpha3.AzureResourceReference, s conversion.Scope) error { - return autoConvert_v1alpha2_AzureResourceReference_To_v1alpha3_AzureResourceReference(in, out, s) -} - -func autoConvert_v1alpha3_AzureResourceReference_To_v1alpha2_AzureResourceReference(in *v1alpha3.AzureResourceReference, out *AzureResourceReference, s conversion.Scope) error { - out.ID = (*string)(unsafe.Pointer(in.ID)) - return nil -} - -// Convert_v1alpha3_AzureResourceReference_To_v1alpha2_AzureResourceReference is an autogenerated conversion function. -func Convert_v1alpha3_AzureResourceReference_To_v1alpha2_AzureResourceReference(in *v1alpha3.AzureResourceReference, out *AzureResourceReference, s conversion.Scope) error { - return autoConvert_v1alpha3_AzureResourceReference_To_v1alpha2_AzureResourceReference(in, out, s) -} - func autoConvert_v1alpha2_BackendPool_To_v1alpha3_BackendPool(in *BackendPool, out *v1alpha3.BackendPool, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go index cd9940f325c..4871cbc6691 100644 --- a/api/v1alpha3/types.go +++ b/api/v1alpha3/types.go @@ -23,22 +23,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// // ResourceSpec defines a generic spec that can used to define Azure resources. -// TODO: ResourceSpec should be removed once concrete specs have been defined for all Azure resources in use. -// type ResourceSpec interface{} - -// TODO: Write type tests - -// AzureResourceReference is a reference to a specific Azure resource by ID -type AzureResourceReference struct { - // ID of resource - // +optional - ID *string `json:"id,omitempty"` - // TODO: Investigate if we should reference resources in other ways -} - -// TODO: Investigate resource filters - // AzureMachineProviderConditionType is a valid value for AzureMachineProviderCondition.Type type AzureMachineProviderConditionType string @@ -157,14 +141,6 @@ type SecurityGroup struct { Tags Tags `json:"tags,omitempty"` } -/* -// TODO -// String returns a string representation of the security group. -func (s *SecurityGroup) String() string { - return fmt.Sprintf("id=%s/name=%s", s.ID, s.Name) -} -*/ - // SecurityGroupProtocol defines the protocol type for a security group rule. type SecurityGroupProtocol string @@ -197,45 +173,10 @@ type IngressRule struct { Destination *string `json:"destination,omitempty"` } -// TODO -// String returns a string representation of the ingress rule. -/* -func (i *IngressRule) String() string { - return fmt.Sprintf("protocol=%s/range=[%d-%d]/description=%s", i.Protocol, i.FromPort, i.ToPort, i.Description) -} -*/ - // IngressRules is a slice of Azure ingress rules for security groups. type IngressRules []*IngressRule -// TODO -// Difference returns the difference between this slice and the other slice. -/* -func (i IngressRules) Difference(o IngressRules) (out IngressRules) { - for _, x := range i { - found := false - for _, y := range o { - sort.Strings(x.CidrBlocks) - sort.Strings(y.CidrBlocks) - sort.Strings(x.SourceSecurityGroupIDs) - sort.Strings(y.SourceSecurityGroupIDs) - if reflect.DeepEqual(x, y) { - found = true - break - } - } - - if !found { - out = append(out, x) - } - } - - return -} -*/ - // PublicIP defines an Azure public IP address. -// TODO: Remove once load balancer is implemented. type PublicIP struct { ID string `json:"id,omitempty"` Name string `json:"name,omitempty"` diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index dc0be6bb61e..b62aab702d2 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -401,26 +401,6 @@ func (in *AzureMarketplaceImage) DeepCopy() *AzureMarketplaceImage { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AzureResourceReference) DeepCopyInto(out *AzureResourceReference) { - *out = *in - if in.ID != nil { - in, out := &in.ID, &out.ID - *out = new(string) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureResourceReference. -func (in *AzureResourceReference) DeepCopy() *AzureResourceReference { - if in == nil { - return nil - } - out := new(AzureResourceReference) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AzureSharedGalleryImage) DeepCopyInto(out *AzureSharedGalleryImage) { *out = *in diff --git a/cloud/services/subnets/subnets.go b/cloud/services/subnets/subnets.go index 19f0b5cfd4d..01291e5c18c 100644 --- a/cloud/services/subnets/subnets.go +++ b/cloud/services/subnets/subnets.go @@ -75,7 +75,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { return errors.New("Invalid Subnet Specification") } if subnet, err := s.Get(ctx, subnetSpec); err == nil { - // TODO: add validation on existing subnet // subnet already exists, skip creation if subnetSpec.Role == infrav1.SubnetControlPlane { subnet.DeepCopyInto(s.Scope.ControlPlaneSubnet()) diff --git a/cloud/services/virtualmachineextensions/virtualmachineextensions.go b/cloud/services/virtualmachineextensions/virtualmachineextensions.go index 01583c0568c..64b937be754 100644 --- a/cloud/services/virtualmachineextensions/virtualmachineextensions.go +++ b/cloud/services/virtualmachineextensions/virtualmachineextensions.go @@ -78,12 +78,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { return errors.Wrapf(err, "cannot create vm extension") } - // TODO: review if can remove this code - // if *vmExt.ProvisioningState != string(compute.ProvisioningStateSucceeded) { - // // If the script failed delete it so it can be retried - // s.Delete(ctx, vmExtSpec) - // } - klog.V(2).Infof("successfully created vm extension %s ", vmExtSpec.Name) return nil } diff --git a/cloud/services/virtualmachines/virtualmachines.go b/cloud/services/virtualmachines/virtualmachines.go index 31529c495ed..0f3d602aa23 100644 --- a/cloud/services/virtualmachines/virtualmachines.go +++ b/cloud/services/virtualmachines/virtualmachines.go @@ -275,7 +275,6 @@ func getResourceNameByID(resourceID string) string { // generateStorageProfile generates a pointer to a compute.StorageProfile which can utilized for VM creation. func generateStorageProfile(vmSpec Spec) (*compute.StorageProfile, error) { - // TODO: Validate parameters before building storage profile storageProfile := &compute.StorageProfile{ OsDisk: &compute.OSDisk{ Name: to.StringPtr(azure.GenerateOSDiskName(vmSpec.Name)), diff --git a/cloud/services/virtualnetworks/virtualnetworks.go b/cloud/services/virtualnetworks/virtualnetworks.go index 92edcf894b1..87554a46eb3 100644 --- a/cloud/services/virtualnetworks/virtualnetworks.go +++ b/cloud/services/virtualnetworks/virtualnetworks.go @@ -89,7 +89,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { s.Scope.V(2).Info("Working on custom vnet", "vnet-id", vnet.ID) } // vnet already exists, cannot update since it's immutable - // TODO: ensure tags & other managed vnet attributes vnet.DeepCopyInto(s.Scope.Vnet()) return nil } diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index d14ce28ae20..0fec77b9d90 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -123,8 +123,6 @@ func (r *AzureClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScop return reconcile.Result{}, errors.Wrap(err, "failed to reconcile cluster services") } - // TODO: We may need to use azureCluster.Status.Network.APIServerIP.IPAddress - // instead when we look at configuring private clusters. if azureCluster.Status.Network.APIServerIP.DNSName == "" { clusterScope.Info("Waiting for API server endpoint to exist") return reconcile.Result{RequeueAfter: 15 * time.Second}, nil diff --git a/controllers/azuremachine_controller.go b/controllers/azuremachine_controller.go index 9a93cb0e2d1..5717e71ec1f 100644 --- a/controllers/azuremachine_controller.go +++ b/controllers/azuremachine_controller.go @@ -217,24 +217,16 @@ func (r *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineSco return reconcile.Result{}, err } - // TODO(ncdc): move this validation logic into a validating webhook - if errs := r.validateUpdate(&machineScope.AzureMachine.Spec, vm); len(errs) > 0 { - agg := kerrors.NewAggregate(errs) - r.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "InvalidUpdate", "Invalid update: %s", agg.Error()) - return reconcile.Result{}, nil - } - // Make sure Spec.ProviderID is always set. machineScope.SetProviderID(fmt.Sprintf("azure:////%s", vm.ID)) - // Proceed to reconcile the AzureMachine state. - machineScope.SetVMState(vm.State) - - // TODO(vincepri): Remove this annotation when clusterctl is no longer relevant. machineScope.SetAnnotation("cluster-api-provider-azure", "true") machineScope.SetAddresses(vm.Addresses) + // Proceed to reconcile the AzureMachine state. + machineScope.SetVMState(vm.State) + switch vm.State { case infrav1.VMStateSucceeded: machineScope.V(2).Info("VM is running", "id", *machineScope.GetVMID()) @@ -306,14 +298,7 @@ func (r *AzureMachineReconciler) reconcileDelete(machineScope *scope.MachineScop return reconcile.Result{}, nil } -// validateUpdate checks that no immutable fields have been updated and -// returns a slice of errors representing attempts to change immutable state. -func (r *AzureMachineReconciler) validateUpdate(spec *infrav1.AzureMachineSpec, i *infrav1.VM) (errs []error) { - // TODO: Add comparison logic for immutable fields - return errs -} - -// AzureClusterToAzureMachine is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation +// AzureClusterToAzureMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation // of AzureMachines. func (r *AzureMachineReconciler) AzureClusterToAzureMachines(o handler.MapObject) []ctrl.Request { result := []ctrl.Request{} diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index 68c38acfaf5..9af34c7e61c 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -42,7 +42,6 @@ const ( ) // azureMachineService are list of services required by cluster actuator, easy to create a fake -// TODO: We should decide if we want to keep this type azureMachineService struct { machineScope *scope.MachineScope clusterScope *scope.ClusterScope diff --git a/hack/tools/go.mod b/hack/tools/go.mod index 55cf90e21e2..b86de5c573f 100644 --- a/hack/tools/go.mod +++ b/hack/tools/go.mod @@ -24,17 +24,3 @@ require ( sigs.k8s.io/kustomize/kustomize/v3 v3.5.4 sigs.k8s.io/testing_frameworks v0.1.2 ) - -replace ( - // TODO(vincepri) Remove the following replace directives, needed for golangci-lint until - // https://github.com/golangci/golangci-lint/issues/595 is fixed. - github.com/go-critic/go-critic v0.0.0-20181204210945-1df300866540 => github.com/go-critic/go-critic v0.0.0-20190526074819-1df300866540 - github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6 => github.com/golangci/errcheck v0.0.0-20181223084120-ef45e06d44b6 - github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 => github.com/golangci/go-tools v0.0.0-20190318060251-af6baa5dc196 - github.com/golangci/gofmt v0.0.0-20181105071733-0b8337e80d98 => github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98 - github.com/golangci/gosec v0.0.0-20180901114220-66fb7fc33547 => github.com/golangci/gosec v0.0.0-20190211064107-66fb7fc33547 - github.com/golangci/ineffassign v0.0.0-20180808204949-42439a7714cc => github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc - github.com/golangci/lint-1 v0.0.0-20180610141402-ee948d087217 => github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217 - github.com/timakin/bodyclose => github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2 - mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34 => mvdan.cc/unparam v0.0.0-20190209190245-fbb59629db34 -) diff --git a/hack/tools/go.sum b/hack/tools/go.sum index edc730e9624..d921ed25151 100644 --- a/hack/tools/go.sum +++ b/hack/tools/go.sum @@ -239,8 +239,6 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2 h1:nh/PMhIaxu+h8NOuhOwT2el9Ed08166oitASyNYqQzs= -github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk= github.com/golangci/check v0.0.0-20180506172741-cfe4005ccda2 h1:23T5iq8rbUYlhpt5DB4XJkc6BU31uODLD1o1gKvZmD0= github.com/golangci/check v0.0.0-20180506172741-cfe4005ccda2/go.mod h1:k9Qvh+8juN+UKMCS/3jFtGICgW8O96FVaZsaxdzDkR4= github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a h1:w8hkcTqaFpzKqonE9uMCefW1WDie15eSP/4MssdenaM= @@ -545,6 +543,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk= +github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e h1:RumXZ56IrCj4CL+g1b9OL/oH0QnsF976bC8xQFYUD5Q= +github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tommy-muehle/go-mnd v1.1.1 h1:4D0wuPKjOTiK2garzuPGGvm4zZ/wLYDOH8TJSABC7KU= diff --git a/scripts/ci-conformance.sh b/scripts/ci-conformance.sh index 17b85ef6e9c..d55b5bd8ed0 100755 --- a/scripts/ci-conformance.sh +++ b/scripts/ci-conformance.sh @@ -124,7 +124,6 @@ run_tests() { } get_logs() { - # TODO collect more logs https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/474 kubectl logs deploy/capz-controller-manager -n capz-system manager > "${ARTIFACTS}/logs/capz-manager.log" || true } diff --git a/test/e2e/generators/capi.go b/test/e2e/generators/capi.go index b1afcaabd70..8be156a6513 100644 --- a/test/e2e/generators/capi.go +++ b/test/e2e/generators/capi.go @@ -49,7 +49,6 @@ func (g *ClusterAPI) releaseYAMLPath() string { // Manifests return the generated components and any error if there is one. func (g *ClusterAPI) Manifests(ctx context.Context) ([]byte, error) { - // TODO: this is not very nice if g.GitRef != "" { kustomize := exec.NewCommand( exec.WithCommand("kustomize"), diff --git a/test/e2e/generators/capz.go b/test/e2e/generators/capz.go index 7797e7fe5a6..a3b35928aab 100644 --- a/test/e2e/generators/capz.go +++ b/test/e2e/generators/capz.go @@ -59,6 +59,5 @@ func expandCredVariables(stdout []byte, creds auth.Creds) []byte { os.Setenv("AZURE_CLIENT_SECRET_B64", base64.StdEncoding.EncodeToString([]byte(creds.ClientSecret))) os.Setenv("AZURE_SUBSCRIPTION_ID_B64", base64.StdEncoding.EncodeToString([]byte(creds.SubscriptionID))) os.Setenv("AZURE_TENANT_ID_B64", base64.StdEncoding.EncodeToString([]byte(creds.TenantID))) - // TODO Figure out a better way of doing this return []byte(os.ExpandEnv(string(stdout))) } diff --git a/test/integration/metacluster_test.go b/test/integration/metacluster_test.go index f623eb89d0f..6d548603a05 100644 --- a/test/integration/metacluster_test.go +++ b/test/integration/metacluster_test.go @@ -47,7 +47,4 @@ var _ = Describe("Metacluster", func() { AfterEach(func() { kindCluster.Teardown() }) - - // TODO: validate that the controller-manager is deployed and the - // types are available }) From e80b344a8d25d42e10142e730b8bd22fd7c800ea Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Thu, 23 Apr 2020 16:12:56 -0700 Subject: [PATCH 04/11] [E2E] Remove capi, cabpk and kcp generators --- test/e2e/azure_suite_test.go | 6 +-- test/e2e/generators/cabpk.go | 56 --------------------------- test/e2e/generators/capi.go | 74 ------------------------------------ test/e2e/generators/kcp.go | 52 ------------------------- 4 files changed, 3 insertions(+), 185 deletions(-) delete mode 100644 test/e2e/generators/cabpk.go delete mode 100644 test/e2e/generators/capi.go delete mode 100644 test/e2e/generators/kcp.go diff --git a/test/e2e/azure_suite_test.go b/test/e2e/azure_suite_test.go index 520d115e140..00f479bc5a1 100644 --- a/test/e2e/azure_suite_test.go +++ b/test/e2e/azure_suite_test.go @@ -120,9 +120,9 @@ var _ = BeforeSuite(func() { waitDeployment(c, "cert-manager", "cert-manager-webhook") // Deploy the CAPI and CABPK components from Cluster API repository, - capi := &generators.ClusterAPI{Version: "v0.3.3"} - cabpk := &generators.Bootstrap{Version: "v0.3.3"} - kcp := &generators.KubeadmControPlane{Version: "v0.3.3"} + capi := &frameworkgenerator.ClusterAPI{Version: "v0.3.3"} + cabpk := &frameworkgenerator.KubeadmBootstrap{Version: "v0.3.3"} + kcp := &frameworkgenerator.KubeadmControlPlane{Version: "v0.3.3"} infra := &generators.Infra{Creds: creds} framework.InstallComponents(ctx, mgmt, capi, cabpk, kcp, infra) diff --git a/test/e2e/generators/cabpk.go b/test/e2e/generators/cabpk.go deleted file mode 100644 index cb527f542dc..00000000000 --- a/test/e2e/generators/cabpk.go +++ /dev/null @@ -1,56 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generators - -import ( - "context" - "fmt" - - "github.com/pkg/errors" - "sigs.k8s.io/cluster-api/test/framework/exec" -) - -type Bootstrap struct { - Version string -} - -// GetName returns the name of the components being generated. -func (g *Bootstrap) GetName() string { - return fmt.Sprintf("Cluster API Bootstrap Provider Kubeadm %s", g.Version) -} - -func (g *Bootstrap) kustomizePath() string { - return fmt.Sprintf("https://github.com/kubernetes-sigs/cluster-api//bootstrap/kubeadm/config") -} - -func (g *Bootstrap) releaseYAMLPath() string { - return fmt.Sprintf("https://github.com/kubernetes-sigs/cluster-api-bootstrap-provider-kubeadm/releases/download/%s/bootstrap-components.yaml", g.Version) -} - -// Manifests return the generated components and any error if there is one. -func (g *Bootstrap) Manifests(ctx context.Context) ([]byte, error) { - kustomize := exec.NewCommand( - exec.WithCommand("kustomize"), - exec.WithArgs("build", g.kustomizePath()), - ) - stdout, stderr, err := kustomize.Run(ctx) - if err != nil { - fmt.Println(string(stderr)) - return nil, errors.WithStack(err) - } - return stdout, nil -} diff --git a/test/e2e/generators/capi.go b/test/e2e/generators/capi.go deleted file mode 100644 index 8be156a6513..00000000000 --- a/test/e2e/generators/capi.go +++ /dev/null @@ -1,74 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generators - -import ( - "context" - "fmt" - "io/ioutil" - "net/http" - - "github.com/pkg/errors" - "sigs.k8s.io/cluster-api/test/framework/exec" -) - -// Generator generates provider components for CAPI -type ClusterAPI struct { - // GitRef defines the git ref. If set, the generator will use kustomize - GitRef string - // Version defines the release version. If GitRef is not set Version must be set and will not use kustomize - Version string -} - -// GetName returns the name of the components being generated. -func (g *ClusterAPI) GetName() string { - return fmt.Sprintf("Cluster API %s", g.Version) -} - -func (g *ClusterAPI) kustomizePath() string { - return fmt.Sprintf("https://github.com/kubernetes-sigs/cluster-api//config/") -} - -func (g *ClusterAPI) releaseYAMLPath() string { - return fmt.Sprintf("https://github.com/kubernetes-sigs/cluster-api/releases/download/%s/cluster-api-components.yaml", g.Version) -} - -// Manifests return the generated components and any error if there is one. -func (g *ClusterAPI) Manifests(ctx context.Context) ([]byte, error) { - if g.GitRef != "" { - kustomize := exec.NewCommand( - exec.WithCommand("kustomize"), - exec.WithArgs("build", g.kustomizePath()), - ) - stdout, stderr, err := kustomize.Run(ctx) - if err != nil { - fmt.Println(string(stderr)) - return nil, errors.WithStack(err) - } - return stdout, nil - } - resp, err := http.Get(g.releaseYAMLPath()) - if err != nil { - return nil, errors.WithStack(err) - } - out, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, errors.WithStack(err) - } - defer resp.Body.Close() - return out, nil -} diff --git a/test/e2e/generators/kcp.go b/test/e2e/generators/kcp.go deleted file mode 100644 index adf9e112b43..00000000000 --- a/test/e2e/generators/kcp.go +++ /dev/null @@ -1,52 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generators - -import ( - "context" - "fmt" - - "github.com/pkg/errors" - "sigs.k8s.io/cluster-api/test/framework/exec" -) - -type KubeadmControPlane struct { - Version string -} - -// GetName returns the name of the components being generated. -func (g *KubeadmControPlane) GetName() string { - return fmt.Sprintf("Cluster API Kubeadm ControPlane Provider %s", g.Version) -} - -func (g *KubeadmControPlane) kustomizePath() string { - return fmt.Sprintf("https://github.com/kubernetes-sigs/cluster-api//controlplane/kubeadm/config") -} - -// Manifests return the generated components and any error if there is one. -func (g *KubeadmControPlane) Manifests(ctx context.Context) ([]byte, error) { - kustomize := exec.NewCommand( - exec.WithCommand("kustomize"), - exec.WithArgs("build", g.kustomizePath()), - ) - stdout, stderr, err := kustomize.Run(ctx) - if err != nil { - fmt.Println(string(stderr)) - return nil, errors.WithStack(err) - } - return stdout, nil -} From 963b0e304c5388f31902b8d27481ebf087b4f35b Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Mon, 27 Apr 2020 12:00:36 -0700 Subject: [PATCH 05/11] conformance: fix Kubeadm version when using CI_VERSION and change default to 1 control plane --- scripts/ci-conformance.sh | 5 +-- ...uster-template-conformance-ci-version.yaml | 12 +++++++ .../patches/ci-version.yaml | 34 ++++++++++++++++++- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/scripts/ci-conformance.sh b/scripts/ci-conformance.sh index d55b5bd8ed0..f08e51fc2e6 100755 --- a/scripts/ci-conformance.sh +++ b/scripts/ci-conformance.sh @@ -72,15 +72,16 @@ create_cluster() { # export cluster template which contains the manifests needed for creating the Azure cluster to run the tests if [[ -n ${CI_VERSION:-} || -n ${USE_CI_ARTIFACTS:-} ]]; then export CLUSTER_TEMPLATE="test/cluster-template-conformance-ci-version.yaml" + export CI_VERSION=${CI_VERSION:-$(curl -sSL https://dl.k8s.io/ci/k8s-master.txt)} + export KUBERNETES_VERSION=${CI_VERSION} else export CLUSTER_TEMPLATE="test/cluster-template-conformance.yaml" fi export CLUSTER_NAME="capz-conformance-$(head /dev/urandom | LC_ALL=C tr -dc a-z0-9 | head -c 6 ; echo '')" # Conformance test suite needs a cluster with at least 2 nodes - export CONTROL_PLANE_MACHINE_COUNT=${CONTROL_PLANE_MACHINE_COUNT:-3} + export CONTROL_PLANE_MACHINE_COUNT=${CONTROL_PLANE_MACHINE_COUNT:-1} export WORKER_MACHINE_COUNT=${WORKER_MACHINE_COUNT:-2} - export CI_VERSION=${CI_VERSION:-$(curl -sSL https://dl.k8s.io/ci/k8s-master.txt)} export REGISTRY=conformance # timestamp is in RFC-3339 format to match kubetest export TIMESTAMP=$(date -u '+%Y-%m-%dT%H:%M:%SZ') diff --git a/templates/test/cluster-template-conformance-ci-version.yaml b/templates/test/cluster-template-conformance-ci-version.yaml index e657f2a44dd..71993eeacaf 100644 --- a/templates/test/cluster-template-conformance-ci-version.yaml +++ b/templates/test/cluster-template-conformance-ci-version.yaml @@ -323,6 +323,12 @@ metadata: spec: template: spec: + image: + marketplace: + offer: capi + publisher: cncf-upstream + sku: k8s-1dot18dot2-ubuntu-1804 + version: 2020.04.17 location: ${AZURE_LOCATION} osDisk: diskSizeGB: 128 @@ -340,6 +346,12 @@ metadata: spec: template: spec: + image: + marketplace: + offer: capi + publisher: cncf-upstream + sku: k8s-1dot18dot2-ubuntu-1804 + version: 2020.04.17 location: ${AZURE_LOCATION} osDisk: diskSizeGB: 30 diff --git a/templates/test/conformance-ci-version/patches/ci-version.yaml b/templates/test/conformance-ci-version/patches/ci-version.yaml index 686bfd7041c..c69e0bf8807 100644 --- a/templates/test/conformance-ci-version/patches/ci-version.yaml +++ b/templates/test/conformance-ci-version/patches/ci-version.yaml @@ -106,6 +106,22 @@ spec: "useInstanceMetadata": true } --- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureMachineTemplate +metadata: + name: "${CLUSTER_NAME}-control-plane" +spec: + template: + spec: + image: + # we use the 1.18.2 image as a workaround there is no published marketplace image for k8s CI versions. + # 1.18.2 binaries and images will get replaced to the desired version by the script above. + marketplace: + publisher: cncf-upstream + offer: capi + sku: k8s-1dot18dot2-ubuntu-1804 + version: "2020.04.17" +--- apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 kind: KubeadmConfigTemplate metadata: @@ -209,4 +225,20 @@ spec: "maximumLoadBalancerRuleCount": 250, "useManagedIdentityExtension": false, "useInstanceMetadata": true - } \ No newline at end of file + } +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureMachineTemplate +metadata: + name: ${CLUSTER_NAME}-md-0 +spec: + template: + spec: + image: + # we use the 1.18.2 image as a workaround there is no published marketplace image for k8s CI versions. + # 1.18.2 binaries and images will get replaced to the desired version by the script above. + marketplace: + publisher: cncf-upstream + offer: capi + sku: k8s-1dot18dot2-ubuntu-1804 + version: "2020.04.17" From 3f8c2d320dc24c61b7278c07a14695ab5bafb49b Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Thu, 23 Apr 2020 13:24:37 -0700 Subject: [PATCH 06/11] Move SSHPublicKey default and validation logic to webhook --- api/v1alpha3/azuremachine_default.go | 44 ++++++++++ api/v1alpha3/azuremachine_default_test.go | 60 +++++++++++++ api/v1alpha3/azuremachine_validation.go | 34 ++++++++ api/v1alpha3/azuremachine_validation_test.go | 65 ++++++++++++++ api/v1alpha3/azuremachine_webhook.go | 22 +++++ api/v1alpha3/azuremachine_webhook_test.go | 84 ++++++++++++++++++- .../virtualmachines/virtualmachines.go | 18 +--- 7 files changed, 307 insertions(+), 20 deletions(-) create mode 100644 api/v1alpha3/azuremachine_default.go create mode 100644 api/v1alpha3/azuremachine_default_test.go create mode 100644 api/v1alpha3/azuremachine_validation.go create mode 100644 api/v1alpha3/azuremachine_validation_test.go diff --git a/api/v1alpha3/azuremachine_default.go b/api/v1alpha3/azuremachine_default.go new file mode 100644 index 00000000000..800ef3155d6 --- /dev/null +++ b/api/v1alpha3/azuremachine_default.go @@ -0,0 +1,44 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha3 + +import ( + "crypto/rand" + "crypto/rsa" + + "github.com/pkg/errors" + "golang.org/x/crypto/ssh" +) + +// SetDefaultSSHPublicKey sets the default SSHPublicKey for an AzureMachine +func (m *AzureMachine) SetDefaultSSHPublicKey() error { + sshKeyData := m.Spec.SSHPublicKey + if sshKeyData == "" { + privateKey, perr := rsa.GenerateKey(rand.Reader, 2048) + if perr != nil { + return errors.Wrap(perr, "Failed to generate private key") + } + + publicRsaKey, perr := ssh.NewPublicKey(&privateKey.PublicKey) + if perr != nil { + return errors.Wrap(perr, "Failed to generate public key") + } + m.Spec.SSHPublicKey = string(ssh.MarshalAuthorizedKey(publicRsaKey)) + } + + return nil +} diff --git a/api/v1alpha3/azuremachine_default_test.go b/api/v1alpha3/azuremachine_default_test.go new file mode 100644 index 00000000000..427fe50cbdd --- /dev/null +++ b/api/v1alpha3/azuremachine_default_test.go @@ -0,0 +1,60 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha3 + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestAzureMachine_SetDefaultSSHPublicKey(t *testing.T) { + g := NewWithT(t) + + type test struct { + machine *AzureMachine + } + + existingPublicKey := "testpublickey" + publicKeyExistTest := test{machine: createMachineWithSSHPublicKey(t, existingPublicKey)} + publicKeyNotExistTest := test{machine: createMachineWithSSHPublicKey(t, "")} + + err := publicKeyExistTest.machine.SetDefaultSSHPublicKey() + g.Expect(err).To(BeNil()) + g.Expect(publicKeyExistTest.machine.Spec.SSHPublicKey).To(Equal(existingPublicKey)) + + err = publicKeyNotExistTest.machine.SetDefaultSSHPublicKey() + g.Expect(err).To(BeNil()) + g.Expect(publicKeyNotExistTest.machine.Spec.SSHPublicKey).To(Not(BeEmpty())) +} + +func createMachineWithSSHPublicKey(t *testing.T, sshPublicKey string) *AzureMachine { + return &AzureMachine{ + Spec: AzureMachineSpec{ + SSHPublicKey: sshPublicKey, + Image: &Image{ + SharedGallery: &AzureSharedGalleryImage{ + SubscriptionID: "SUB123", + ResourceGroup: "RG123", + Name: "NAME123", + Gallery: "GALLERY1", + Version: "1.0.0", + }, + }, + }, + } +} diff --git a/api/v1alpha3/azuremachine_validation.go b/api/v1alpha3/azuremachine_validation.go new file mode 100644 index 00000000000..e10061284d5 --- /dev/null +++ b/api/v1alpha3/azuremachine_validation.go @@ -0,0 +1,34 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha3 + +import ( + "golang.org/x/crypto/ssh" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// ValidateSSHKey validates an SSHKey +func ValidateSSHKey(sshKey string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if _, _, _, _, err := ssh.ParseAuthorizedKey([]byte(sshKey)); err != nil { + allErrs = append(allErrs, field.Required(fldPath, "the SSH public key is not valid")) + return allErrs + } + + return allErrs +} diff --git a/api/v1alpha3/azuremachine_validation_test.go b/api/v1alpha3/azuremachine_validation_test.go new file mode 100644 index 00000000000..96d67b60f1d --- /dev/null +++ b/api/v1alpha3/azuremachine_validation_test.go @@ -0,0 +1,65 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha3 + +import ( + "crypto/rand" + "crypto/rsa" + "testing" + + . "github.com/onsi/gomega" + "golang.org/x/crypto/ssh" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestAzureMachine_ValidateSSHKey(t *testing.T) { + g := NewWithT(t) + + tests := []struct { + name string + sshKey string + wantErr bool + }{ + { + name: "valid ssh key", + sshKey: generateSSHPublicKey(), + wantErr: false, + }, + { + name: "invalid ssh key", + sshKey: "invalid ssh key", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := ValidateSSHKey(tc.sshKey, field.NewPath("sshPublicKey")) + if tc.wantErr { + g.Expect(err).ToNot(HaveLen(0)) + } else { + g.Expect(err).To(HaveLen(0)) + } + }) + } +} + +func generateSSHPublicKey() string { + privateKey, _ := rsa.GenerateKey(rand.Reader, 2048) + publicRsaKey, _ := ssh.NewPublicKey(&privateKey.PublicKey) + return string(ssh.MarshalAuthorizedKey(publicRsaKey)) +} diff --git a/api/v1alpha3/azuremachine_webhook.go b/api/v1alpha3/azuremachine_webhook.go index be86388eee5..54018f2425d 100644 --- a/api/v1alpha3/azuremachine_webhook.go +++ b/api/v1alpha3/azuremachine_webhook.go @@ -49,6 +49,12 @@ func (m *AzureMachine) ValidateCreate() error { m.Name, errs) } + if errs := ValidateSSHKey(m.Spec.SSHPublicKey, field.NewPath("sshPublicKey")); len(errs) > 0 { + return apierrors.NewInvalid( + GroupVersion.WithKind("AzureMachine").GroupKind(), + m.Name, errs) + } + return nil } @@ -56,6 +62,12 @@ func (m *AzureMachine) ValidateCreate() error { func (m *AzureMachine) ValidateUpdate(old runtime.Object) error { machinelog.Info("validate update", "name", m.Name) + if errs := ValidateSSHKey(m.Spec.SSHPublicKey, field.NewPath("sshPublicKey")); len(errs) > 0 { + return apierrors.NewInvalid( + GroupVersion.WithKind("AzureMachine").GroupKind(), + m.Name, errs) + } + return nil } @@ -65,3 +77,13 @@ func (m *AzureMachine) ValidateDelete() error { return nil } + +// Default implements webhookutil.defaulter so a webhook will be registered for the type +func (m *AzureMachine) Default() { + machinelog.Info("default", "name", m.Name) + + err := m.SetDefaultSSHPublicKey() + if err != nil { + machinelog.Error(err, "SetDefaultSshPublicKey failed") + } +} diff --git a/api/v1alpha3/azuremachine_webhook_test.go b/api/v1alpha3/azuremachine_webhook_test.go index 8784b9d038d..a8c08416cd9 100644 --- a/api/v1alpha3/azuremachine_webhook_test.go +++ b/api/v1alpha3/azuremachine_webhook_test.go @@ -22,6 +22,8 @@ import ( . "github.com/onsi/gomega" ) +var validSSHPublicKey = generateSSHPublicKey() + func TestAzureMachine_ValidateCreate(t *testing.T) { g := NewWithT(t) @@ -60,6 +62,21 @@ func TestAzureMachine_ValidateCreate(t *testing.T) { machine: createMachineWithImageByID(t, ""), wantErr: true, }, + { + name: "azuremachine with valid SSHPublicKey", + machine: createMachineWithSSHPublicKey(t, validSSHPublicKey), + wantErr: false, + }, + { + name: "azuremachine without SSHPublicKey", + machine: createMachineWithSSHPublicKey(t, ""), + wantErr: true, + }, + { + name: "azuremachine with invalid SSHPublicKey", + machine: createMachineWithSSHPublicKey(t, "invalid ssh key"), + wantErr: true, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -73,6 +90,64 @@ func TestAzureMachine_ValidateCreate(t *testing.T) { } } +func TestAzureMachine_ValidateUpdate(t *testing.T) { + g := NewWithT(t) + + tests := []struct { + name string + oldMachine *AzureMachine + machine *AzureMachine + wantErr bool + }{ + { + name: "azuremachine with valid SSHPublicKey", + oldMachine: createMachineWithSSHPublicKey(t, ""), + machine: createMachineWithSSHPublicKey(t, validSSHPublicKey), + wantErr: false, + }, + { + name: "azuremachine without SSHPublicKey", + oldMachine: createMachineWithSSHPublicKey(t, ""), + machine: createMachineWithSSHPublicKey(t, ""), + wantErr: true, + }, + { + name: "azuremachine with invalid SSHPublicKey", + oldMachine: createMachineWithSSHPublicKey(t, ""), + machine: createMachineWithSSHPublicKey(t, "invalid ssh key"), + wantErr: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.machine.ValidateUpdate(tc.oldMachine) + if tc.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +func TestAzureMachine_Default(t *testing.T) { + g := NewWithT(t) + + type test struct { + machine *AzureMachine + } + + existingPublicKey := validSSHPublicKey + publicKeyExistTest := test{machine: createMachineWithSSHPublicKey(t, existingPublicKey)} + publicKeyNotExistTest := test{machine: createMachineWithSSHPublicKey(t, "")} + + publicKeyExistTest.machine.Default() + g.Expect(publicKeyExistTest.machine.Spec.SSHPublicKey).To(Equal(existingPublicKey)) + + publicKeyNotExistTest.machine.Default() + g.Expect(publicKeyNotExistTest.machine.Spec.SSHPublicKey).To(Not(BeEmpty())) +} + func createMachineWithSharedImage(t *testing.T, subscriptionID, resourceGroup, name, gallery, version string) *AzureMachine { image := &Image{ SharedGallery: &AzureSharedGalleryImage{ @@ -86,7 +161,8 @@ func createMachineWithSharedImage(t *testing.T, subscriptionID, resourceGroup, n return &AzureMachine{ Spec: AzureMachineSpec{ - Image: image, + Image: image, + SSHPublicKey: validSSHPublicKey, }, } @@ -104,7 +180,8 @@ func createMachineWithtMarketPlaceImage(t *testing.T, publisher, offer, sku, ver return &AzureMachine{ Spec: AzureMachineSpec{ - Image: image, + Image: image, + SSHPublicKey: validSSHPublicKey, }, } } @@ -116,7 +193,8 @@ func createMachineWithImageByID(t *testing.T, imageID string) *AzureMachine { return &AzureMachine{ Spec: AzureMachineSpec{ - Image: image, + Image: image, + SSHPublicKey: validSSHPublicKey, }, } } diff --git a/cloud/services/virtualmachines/virtualmachines.go b/cloud/services/virtualmachines/virtualmachines.go index 31529c495ed..f13cef91884 100644 --- a/cloud/services/virtualmachines/virtualmachines.go +++ b/cloud/services/virtualmachines/virtualmachines.go @@ -19,7 +19,6 @@ package virtualmachines import ( "context" "crypto/rand" - "crypto/rsa" "encoding/base64" "fmt" "strings" @@ -27,7 +26,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute" "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" - "golang.org/x/crypto/ssh" corev1 "k8s.io/api/core/v1" "k8s.io/klog" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" @@ -96,20 +94,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { klog.V(2).Infof("creating VM %s ", vmSpec.Name) - sshKeyData := vmSpec.SSHKeyData - if sshKeyData == "" { - privateKey, perr := rsa.GenerateKey(rand.Reader, 2048) - if perr != nil { - return errors.Wrap(perr, "Failed to generate private key") - } - - publicRsaKey, perr := ssh.NewPublicKey(&privateKey.PublicKey) - if perr != nil { - return errors.Wrap(perr, "Failed to generate public key") - } - sshKeyData = string(ssh.MarshalAuthorizedKey(publicRsaKey)) - } - // Make sure to use the MachineScope here to get the merger of AzureCluster and AzureMachine tags additionalTags := s.MachineScope.AdditionalTags() // Set the cloud provider tag @@ -139,7 +123,7 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { PublicKeys: &[]compute.SSHPublicKey{ { Path: to.StringPtr(fmt.Sprintf("/home/%s/.ssh/authorized_keys", azure.DefaultUserName)), - KeyData: to.StringPtr(sshKeyData), + KeyData: to.StringPtr(vmSpec.SSHKeyData), }, }, }, From 80b693415ed912d9ea7e9391c49200b58f973370 Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Mon, 27 Apr 2020 16:59:46 -0700 Subject: [PATCH 07/11] conformance: fix kubectl path --- scripts/ci-conformance.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/ci-conformance.sh b/scripts/ci-conformance.sh index f08e51fc2e6..c91f40f7b87 100755 --- a/scripts/ci-conformance.sh +++ b/scripts/ci-conformance.sh @@ -57,8 +57,7 @@ build_k8s() { cp -f "${PWD}/bazel-bin/test/e2e/e2e.test" "${PWD}/_output/bin/e2e.test" # workaround for mac os cp -f "${PWD}/bazel-bin/vendor/github.com/onsi/ginkgo/ginkgo/darwin_amd64_stripped/ginkgo" "${PWD}/_output/bin/ginkgo" || true - export KUBECTL_PATH="$(dirname "$(find "${PWD}/bazel-bin/" -name kubectl -type f)")/kubectl" - PATH="${KUBECTL_PATH}:${PATH}" + PATH="$(dirname "$(find "${PWD}/bazel-bin/" -name kubectl -type f)"):${PATH}" export PATH # attempt to release some memory after building From ca5226f0f202afb3d6961783e9bfb1b7881b538f Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Tue, 28 Apr 2020 13:05:59 -0700 Subject: [PATCH 08/11] Remove kustomize dependency from create-workload-cluster target --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4c93d3dcf03..c9bce22f043 100644 --- a/Makefile +++ b/Makefile @@ -341,7 +341,7 @@ create-management-cluster: $(KUSTOMIZE) $(ENVSUBST) @echo 'Set kubectl context to the kind management cluster by running "kubectl config set-context kind-capz"' .PHONY: create-workload-cluster -create-workload-cluster: $(KUSTOMIZE) $(ENVSUBST) +create-workload-cluster: $(ENVSUBST) # Create workload Cluster. $(ENVSUBST) < $(TEMPLATES_DIR)/$(CLUSTER_TEMPLATE) | kubectl apply -f - From eedc11b11341504c3b21122339362ff720a2d2dd Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Tue, 28 Apr 2020 13:06:31 -0700 Subject: [PATCH 09/11] set nounset and pipefail in kind script --- scripts/kind-with-registry.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/kind-with-registry.sh b/scripts/kind-with-registry.sh index 8568e0b9071..4e24f512736 100755 --- a/scripts/kind-with-registry.sh +++ b/scripts/kind-with-registry.sh @@ -14,6 +14,8 @@ # limitations under the License. set -o errexit +set -o nounset +set -o pipefail # desired cluster name; default is "kind" KIND_CLUSTER_NAME="${KIND_CLUSTER_NAME:-capz}" From 15ef62f38233342a95748e0b3631cfedad9920f4 Mon Sep 17 00:00:00 2001 From: Matt Boersma Date: Tue, 28 Apr 2020 15:05:11 -0600 Subject: [PATCH 10/11] docs: fix a link and some typos in developer doc --- docs/development.md | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/docs/development.md b/docs/development.md index 18870e05062..411e07f450b 100644 --- a/docs/development.md +++ b/docs/development.md @@ -42,22 +42,23 @@ 1. Install [go][go] - Get the latest patch version for go v1.13. 2. Install [jq][jq] - - `brew install jq` on MacOS. + - `brew install jq` on macOS. - `chocolatey install jq` on Windows. - - `sudo apt-get install jq` on Linux. + - `sudo apt install jq` on Ubuntu Linux. 3. Install [gettext][gettext] package - - `brew install gettext && brew link --force gettext` on MacOS. - - [install instructions](gettextwindows) on Windows. + - `brew install gettext && brew link --force gettext` on macOS. + - [install instructions][gettextwindows] on Windows. + - `sudo apt install gettext` on Ubuntu Linux. 4. Install [KIND][kind] - `GO111MODULE="on" go get sigs.k8s.io/kind@v0.7.0`. 5. Install [Kustomize][kustomize] - - `brew install kustomize` on MacOs. + - `brew install kustomize` on macOS. - `choco install kustomize` on Windows. - [install instructions][kustomizelinux] on Linux 6. Configure Python 2.7+ with [pyenv][pyenv] if your default is Python 3.x. 7. Install make. 8. Install [timeout][timeout] - - `brew install coreutils` on MacOS. + - `brew install coreutils` on macOS. ### Get the source @@ -96,14 +97,16 @@ To pin a new dependency: - Run `go get @`. - (Optional) Add a `replace` statement in `go.mod`. -A few Makefile and scripts are offered to work with go modules: +Makefile targets and scripts are offered to work with go modules: -- `hack/ensure-go.sh` file checks that the Go version and environment variables are properly set. +- `make verify-modules` checks whether go module files are out of date. +- `make modules` runs `go mod tidy` to ensure proper vendoring. +- `hack/ensure-go.sh` checks that the Go version and environment variables are properly set. ### Setting up the environment Your environment must have the Azure credentials as outlined in the [getting -started prerequisites section](./getting-started.md#Prerequisites) +started prerequisites](./getting-started.md#Prerequisites) section. ### Using Tilt @@ -150,20 +153,20 @@ make kind-reset If you want to develop in both CAPI and CAPZ at the same time, then this is the path for you. -To use [Tilt](https://tilt.dev/) for a simplified development workflow, follow the [instructions](https://cluster-api.sigs.k8s.io/developer/tilt.html) in the cluster-api repo. The instructions will walk you through cloning the Cluster API (capi) repository and configuring Tilt to use `kind` to deploy the cluster api management components. +To use [Tilt](https://tilt.dev/) for a simplified development workflow, follow the [instructions](https://cluster-api.sigs.k8s.io/developer/tilt.html) in the cluster-api repo. The instructions will walk you through cloning the Cluster API (CAPI) repository and configuring Tilt to use `kind` to deploy the cluster api management components. -> you may wish to checkout out the correct version of capi to match the [version used in capz](go.mod) +> you may wish to checkout out the correct version of CAPI to match the [version used in CAPZ](go.mod) Note that `tilt up` will be run from the `cluster-api repository` directory and the `tilt-settings.json` file will point back to the `cluster-api-provider-azure` repository directory. Any changes you make to the source code in `cluster-api` or `cluster-api-provider-azure` repositories will automatically redeployed to the `kind` cluster. -After you have cloned both repositories your folder structure should look like: +After you have cloned both repositories, your folder structure should look like: ```tree |-- src/cluster-api-provider-azure |-- src/cluster-api (run `tilt up` here) ``` -After configuring the environment variables, you can run the following to generate you `tilt-settings.json` file: +After configuring the environment variables, run the following to generate your `tilt-settings.json` file: ```shell cat < tilt-settings.json @@ -274,8 +277,7 @@ export AZURE_SSH_PUBLIC_KEY=$(cat "${SSH_KEY_FILE}.pub" | base64 | tr -d '\r\n') Ensure dev environment has been reset: ```bash -make clean -make kind-reset +make clean kind-reset ``` Create the cluster: @@ -294,7 +296,7 @@ time kubectl get po -o wide --all-namespaces -w # Watch pod creation until azure kubectl logs -n capz-system azure-provider-controller-manager-0 manager -f # Follow the controller logs ``` -An error such as the following in the manager could point to a miss match between a current capi with old capz version: +An error such as the following in the manager could point to a mismatch between a current CAPI and an old CAPZ version: ``` E0320 23:33:33.288073 1 controller.go:258] controller-runtime/controller "msg"="Reconciler error" "error"="failed to create AzureMachine VM: failed to create nic capz-cluster-control-plane-7z8ng-nic for machine capz-cluster-control-plane-7z8ng: unable to determine NAT rule for control plane network interface: strconv.Atoi: parsing \"capz-cluster-control-plane-7z8ng\": invalid syntax" "controller"="azuremachine" "request"={"Namespace":"default","Name":"capz-cluster-control-plane-7z8ng"} @@ -323,7 +325,7 @@ Kubernetes cluster, nor do they have external dependencies. #### Mocks -Mocks for the services tests are generated using [GoMock][gomock] +Mocks for the services tests are generated using [GoMock][gomock]. To generate the mocks you can run From 2f7969e90dce2911d34f516b1f2966c4e93a6011 Mon Sep 17 00:00:00 2001 From: Matt Boersma Date: Tue, 28 Apr 2020 16:00:44 -0600 Subject: [PATCH 11/11] docs: fix kubectl link in getting started doc --- docs/getting-started.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index b54e45151b6..734f6b4d736 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -21,7 +21,7 @@ ### Requirements -- Linux or MacOS (Windows isn't supported at the moment) +- Linux or macOS (Windows isn't supported at the moment) - A [Microsoft Azure account](https://azure.microsoft.com/en-us/) - Install the [Azure CLI](https://docs.microsoft.com/en-us/cli/azure/install-azure-cli?view=azure-cli-latest) - Install the [Kubernetes CLI](https://kubernetes.io/docs/tasks/tools/install-kubectl/) @@ -90,7 +90,7 @@ You can also [build your own image](https://image-builder.sigs.k8s.io/capi/provi ### Bootstrap running, but resources aren't being created -Logs can be tailed using [`kubectl`][kubectl]: +Logs can be tailed using [`kubectl`][kubectl-logs]: ```bash kubectl logs azure-provider-controller-manager-0 -n azure-provider-system -f @@ -105,3 +105,5 @@ You can check the custom script logs by SSHing into the VM created and reading ` ## Building from master If you're interested in developing cluster-api-provider-azure and getting the latest version from `master`, please follow the [development guide][development]. + +[kubectl-logs]: https://kubernetes.io/docs/reference/kubectl/cheatsheet/#interacting-with-running-pods