From 1e93c54d20c8ac7f2e62bbf9aaf8c9ce4fe8228b Mon Sep 17 00:00:00 2001 From: Steven Gettys Date: Mon, 9 May 2022 09:59:47 -0700 Subject: [PATCH 1/7] chore: scaffolding for parameterset crd Signed-off-by: Steven Gettys --- PROJECT | 8 + api/v1/parameterset_types.go | 114 ++++++++ api/v1/parameterset_types_test.go | 105 +++++++ api/v1/testdata/credential-set.yaml | 6 +- api/v1/testdata/parameter-set.yaml | 7 + api/v1/zz_generated.deepcopy.go | 89 ++++++ config/crd/kustomization.yaml | 3 + .../patches/cainjection_in_parametersets.yaml | 7 + .../crd/patches/webhook_in_parametersets.yaml | 16 + config/rbac/parameterset_editor_role.yaml | 24 ++ config/rbac/parameterset_viewer_role.yaml | 20 ++ config/samples/_v1_parameterset.yaml | 6 + controllers/parameterset_controller.go | 46 +++ controllers/parameterset_controller_test.go | 275 ++++++++++++++++++ main.go | 7 + 15 files changed, 730 insertions(+), 3 deletions(-) create mode 100644 api/v1/parameterset_types.go create mode 100644 api/v1/parameterset_types_test.go create mode 100644 api/v1/testdata/parameter-set.yaml create mode 100644 config/crd/patches/cainjection_in_parametersets.yaml create mode 100644 config/crd/patches/webhook_in_parametersets.yaml create mode 100644 config/rbac/parameterset_editor_role.yaml create mode 100644 config/rbac/parameterset_viewer_role.yaml create mode 100644 config/samples/_v1_parameterset.yaml create mode 100644 controllers/parameterset_controller.go create mode 100644 controllers/parameterset_controller_test.go diff --git a/PROJECT b/PROJECT index cf648b58..77555d2b 100644 --- a/PROJECT +++ b/PROJECT @@ -39,4 +39,12 @@ resources: kind: CredentialSet path: get.porter.sh/operator/api/v1 version: v1 +- api: + crdVersion: v1 + namespaced: true + controller: true + domain: porter.sh + kind: ParameterSet + path: get.porter.sh/operator/api/v1 + version: v1 version: "3" diff --git a/api/v1/parameterset_types.go b/api/v1/parameterset_types.go new file mode 100644 index 00000000..9c9ffa84 --- /dev/null +++ b/api/v1/parameterset_types.go @@ -0,0 +1,114 @@ +package v1 + +import ( + "github.com/pkg/errors" + "gopkg.in/yaml.v3" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// SERIALIZATION NOTE: +// * json tags are required for Kubernetes. Any new fields you add must have json tags for the fields to be serialized. +// * yaml tags are required for Porter. Any new fields you add must have yaml tags for the fields to be serialized. + +// Parameter defines an element in a ParameterSet +type Parameter struct { + // Name is the bundle parameter name + Name string `json:"name" yaml:"name"` + + //Source is the bundle parameter source + //supported: secret, value + //unsupported: file path(via configMap), env var, shell cmd + Source ParameterSource `json:"source" yaml:"source"` +} + +type ParameterSource struct { + // Secret is a parameter source using a secret plugin + Secret string `json:"secret,omitempty" yaml:"secret,omitempty"` + // Value is a paremeter source using plaintext value + Value string `json:"value,omitempty" yaml:"value,omitempty"` +} + +// ParameterSetSpec defines the desired state of ParameterSet +type ParameterSetSpec struct { + // AgentConfig is the name of an AgentConfig to use instead of the AgentConfig defined at the namespace or system level. + // +optional + AgentConfig *corev1.LocalObjectReference `json:"agentConfig,omitempty" yaml:"-"` + + // PorterConfig is the name of a PorterConfig to use instead of the PorterConfig defined at the namespace or system level. + PorterConfig *corev1.LocalObjectReference `json:"porterConfig,omitempty" yaml:"-"` + // + // These are fields from the Porter parameter set resource. + // Your goal is that someone can copy/paste a resource from Porter into the + // spec and have it work. So be consistent! + // + + // SchemaVersion is the version of the parameter set state schema. + SchemaVersion string `json:"schemaVersion" yaml:"schemaVersion"` + + // Name is the name of the parameter set in Porter. Immutable. + Name string `json:"name" yaml:"name"` + + // Namespace (in Porter) where the parameter set is defined. + Namespace string `json:"namespace" yaml:"namespace"` + // Parameters list of bundle parameters in the parameter set. + Parameters []Parameter `json:"parameters" yaml:"parameters"` +} + +func (ps ParameterSetSpec) ToPorterDocument() ([]byte, error) { + b, err := yaml.Marshal(ps) + return b, errors.Wrap(err, "error converting the ParameterSet spec into its Porter resource representation") +} + +// ParameterSetStatus defines the observed state of ParameterSet +type ParameterSetStatus struct { + PorterResourceStatus `json:",inline"` +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status + +// ParameterSet is the Schema for the parametersets API +type ParameterSet struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ParameterSetSpec `json:"spec,omitempty"` + Status ParameterSetStatus `json:"status,omitempty"` +} + +func (ps *ParameterSet) GetStatus() PorterResourceStatus { + return ps.Status.PorterResourceStatus +} + +func (ps *ParameterSet) SetStatus(value PorterResourceStatus) { + ps.Status.PorterResourceStatus = value +} + +// GetRetryLabelValue returns a value that is safe to use +// as a label value and represents the retry annotation used +// to trigger reconciliation. +func (ps *ParameterSet) GetRetryLabelValue() string { + return getRetryLabelValue(ps.Annotations) +} + +// SetRetryAnnotation flags the resource to retry its last operation. +func (ps *ParameterSet) SetRetryAnnotation(retry string) { + if ps.Annotations == nil { + ps.Annotations = make(map[string]string, 1) + } + ps.Annotations[AnnotationRetry] = retry +} + +//+kubebuilder:object:root=true + +// ParameterSetList contains a list of ParameterSet +type ParameterSetList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ParameterSet `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ParameterSet{}, &ParameterSetList{}) +} diff --git a/api/v1/parameterset_types_test.go b/api/v1/parameterset_types_test.go new file mode 100644 index 00000000..a6625bfc --- /dev/null +++ b/api/v1/parameterset_types_test.go @@ -0,0 +1,105 @@ +package v1 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + portertest "get.porter.sh/porter/pkg/test" + portertests "get.porter.sh/porter/tests" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestParameterSetSpec_ToPorterDocument(t *testing.T) { + wantGoldenFile := "testdata/parameter-set.yaml" + type fields struct { + AgentConfig *corev1.LocalObjectReference + PorterConfig *corev1.LocalObjectReference + SchemaVersion string + Name string + Namespace string + Parameters []Parameter + } + tests := []struct { + name string + fields fields + wantFile string + wantErrMsg string + }{ + { + name: "golden file test", + fields: fields{SchemaVersion: "1.0.1", + Name: "porter-test-me", + Namespace: "dev", + Parameters: []Parameter{{ + Name: "param1", + Source: ParameterSource{Value: "test-param"}, + }, + }, + }, + wantFile: wantGoldenFile, + wantErrMsg: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cs := ParameterSetSpec{ + AgentConfig: tt.fields.AgentConfig, + PorterConfig: tt.fields.PorterConfig, + SchemaVersion: tt.fields.SchemaVersion, + Name: tt.fields.Name, + Namespace: tt.fields.Namespace, + Parameters: tt.fields.Parameters, + } + got, err := cs.ToPorterDocument() + if tt.wantErrMsg == "" { + require.NoError(t, err) + portertest.CompareGoldenFile(t, "testdata/parameter-set.yaml", string(got)) + } else { + portertests.RequireErrorContains(t, err, tt.wantErrMsg) + } + }) + } +} + +func TestParameterSet_SetRetryAnnotation(t *testing.T) { + type fields struct { + TypeMeta metav1.TypeMeta + ObjectMeta metav1.ObjectMeta + Spec ParameterSetSpec + Status ParameterSetStatus + } + type args struct { + retry string + } + tests := []struct { + name string + fields fields + args args + }{ + { + name: "set retry 1", + fields: fields{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: ParameterSetSpec{}, + Status: ParameterSetStatus{}, + }, + args: args{retry: "1"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cs := &ParameterSet{ + TypeMeta: tt.fields.TypeMeta, + ObjectMeta: tt.fields.ObjectMeta, + Spec: tt.fields.Spec, + Status: tt.fields.Status, + } + cs.SetRetryAnnotation(tt.args.retry) + assert.Equal(t, tt.args.retry, cs.Annotations[AnnotationRetry]) + }) + } +} diff --git a/api/v1/testdata/credential-set.yaml b/api/v1/testdata/credential-set.yaml index 4cbb8a2c..6b7df3c8 100644 --- a/api/v1/testdata/credential-set.yaml +++ b/api/v1/testdata/credential-set.yaml @@ -2,6 +2,6 @@ schemaVersion: 1.0.1 name: porter-test-me namespace: dev credentials: - - name: insecureValue - source: - secret: test-secret + - name: insecureValue + source: + secret: test-secret diff --git a/api/v1/testdata/parameter-set.yaml b/api/v1/testdata/parameter-set.yaml new file mode 100644 index 00000000..7b15024a --- /dev/null +++ b/api/v1/testdata/parameter-set.yaml @@ -0,0 +1,7 @@ +schemaVersion: 1.0.1 +name: porter-test-me +namespace: dev +parameters: + - name: param1 + source: + value: test-param diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 9be75607..7a2e3434 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -518,6 +518,95 @@ func (in *OCIReferenceParts) DeepCopy() *OCIReferenceParts { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ParameterSet) DeepCopyInto(out *ParameterSet) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSet. +func (in *ParameterSet) DeepCopy() *ParameterSet { + if in == nil { + return nil + } + out := new(ParameterSet) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ParameterSet) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ParameterSetList) DeepCopyInto(out *ParameterSetList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ParameterSet, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSetList. +func (in *ParameterSetList) DeepCopy() *ParameterSetList { + if in == nil { + return nil + } + out := new(ParameterSetList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ParameterSetList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ParameterSetSpec) DeepCopyInto(out *ParameterSetSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSetSpec. +func (in *ParameterSetSpec) DeepCopy() *ParameterSetSpec { + if in == nil { + return nil + } + out := new(ParameterSetSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ParameterSetStatus) DeepCopyInto(out *ParameterSetStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSetStatus. +func (in *ParameterSetStatus) DeepCopy() *ParameterSetStatus { + if in == nil { + return nil + } + out := new(ParameterSetStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PluginConfig) DeepCopyInto(out *PluginConfig) { *out = *in diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index d9b501ed..49901298 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -7,6 +7,7 @@ resources: - bases/porter.sh_porterconfigs.yaml - bases/porter.sh_agentactions.yaml - bases/porter.sh_credentialsets.yaml +- bases/porter.sh_parametersets.yaml # +kubebuilder:scaffold:crdkustomizeresource patchesStrategicMerge: @@ -16,6 +17,7 @@ patchesStrategicMerge: #- patches/webhook_in_porterconfigs.yaml #- patches/webhook_in_credentialsets.yaml #- patches/webhook_in_agentactions.yaml +#- patches/webhook_in_parametersets.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. @@ -24,6 +26,7 @@ patchesStrategicMerge: #- patches/cainjection_in_porterconfigs.yaml #- patches/cainjection_in_credentialsets.yaml #- patches/cainjection_in_agentactions.yaml +#- patches/cainjection_in_parametersets.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch # the following config is for teaching kustomize how to do kustomization for CRDs. diff --git a/config/crd/patches/cainjection_in_parametersets.yaml b/config/crd/patches/cainjection_in_parametersets.yaml new file mode 100644 index 00000000..88480fba --- /dev/null +++ b/config/crd/patches/cainjection_in_parametersets.yaml @@ -0,0 +1,7 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: parametersets.porter.sh diff --git a/config/crd/patches/webhook_in_parametersets.yaml b/config/crd/patches/webhook_in_parametersets.yaml new file mode 100644 index 00000000..087c75ce --- /dev/null +++ b/config/crd/patches/webhook_in_parametersets.yaml @@ -0,0 +1,16 @@ +# The following patch enables a conversion webhook for the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: parametersets.porter.sh +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/rbac/parameterset_editor_role.yaml b/config/rbac/parameterset_editor_role.yaml new file mode 100644 index 00000000..0c79a446 --- /dev/null +++ b/config/rbac/parameterset_editor_role.yaml @@ -0,0 +1,24 @@ +# permissions for end users to edit parametersets. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: parameterset-editor-role +rules: +- apiGroups: + - porter.sh + resources: + - parametersets + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - porter.sh + resources: + - parametersets/status + verbs: + - get diff --git a/config/rbac/parameterset_viewer_role.yaml b/config/rbac/parameterset_viewer_role.yaml new file mode 100644 index 00000000..7b62f866 --- /dev/null +++ b/config/rbac/parameterset_viewer_role.yaml @@ -0,0 +1,20 @@ +# permissions for end users to view parametersets. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: parameterset-viewer-role +rules: +- apiGroups: + - porter.sh + resources: + - parametersets + verbs: + - get + - list + - watch +- apiGroups: + - porter.sh + resources: + - parametersets/status + verbs: + - get diff --git a/config/samples/_v1_parameterset.yaml b/config/samples/_v1_parameterset.yaml new file mode 100644 index 00000000..bbeff556 --- /dev/null +++ b/config/samples/_v1_parameterset.yaml @@ -0,0 +1,6 @@ +apiVersion: porter.sh/v1 +kind: ParameterSet +metadata: + name: parameterset-sample +spec: + # TODO(user): Add fields here diff --git a/controllers/parameterset_controller.go b/controllers/parameterset_controller.go new file mode 100644 index 00000000..7c4e1d07 --- /dev/null +++ b/controllers/parameterset_controller.go @@ -0,0 +1,46 @@ +package controllers + +import ( + "context" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + portershv1 "get.porter.sh/operator/api/v1" +) + +// ParameterSetReconciler reconciles a ParameterSet object +type ParameterSetReconciler struct { + client.Client + Scheme *runtime.Scheme +} + +//+kubebuilder:rbac:groups=porter.sh,resources=parametersets,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=porter.sh,resources=parametersets/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=porter.sh,resources=parametersets/finalizers,verbs=update + +// Reconcile is part of the main kubernetes reconciliation loop which aims to +// move the current state of the cluster closer to the desired state. +// TODO(user): Modify the Reconcile function to compare the state specified by +// the ParameterSet object against the actual cluster state, and then +// perform operations to make the cluster state reflect the state specified by +// the user. +// +// For more details, check Reconcile and its Result here: +// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.2/pkg/reconcile +func (r *ParameterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + _ = log.FromContext(ctx) + + // TODO(user): your logic here + + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ParameterSetReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&portershv1.ParameterSet{}). + Complete(r) +} diff --git a/controllers/parameterset_controller_test.go b/controllers/parameterset_controller_test.go new file mode 100644 index 00000000..89d42af1 --- /dev/null +++ b/controllers/parameterset_controller_test.go @@ -0,0 +1,275 @@ +package controllers + +import ( + "context" + "testing" + "time" + + porterv1 "get.porter.sh/operator/api/v1" + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/pointer" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +func TestParameterSetReconiler_Reconcile(t *testing.T) { + ctx := context.Background() + + namespace := "test" + name := "mybuns" + testdata := []client.Object{ + &porterv1.CredentialSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: name, Generation: 1}, + }, + } + controller := setupParameterSetController(testdata...) + + var ps porterv1.ParameterSet + triggerReconcile := func() { + fullname := types.NamespacedName{Namespace: namespace, Name: name} + key := client.ObjectKey{Namespace: namespace, Name: name} + request := controllerruntime.Request{ + NamespacedName: fullname, + } + result, err := controller.Reconcile(ctx, request) + require.NoError(t, err) + require.True(t, result.IsZero()) + + err = controller.Get(ctx, key, &ps) + if !apierrors.IsNotFound(err) { + require.NoError(t, err) + } + } + triggerReconcile() + + // Verify the credential set was picked up and the status initialized + assert.Equal(t, porterv1.PhaseUnknown, ps.Status.Phase, "New resources should be initialized to Phase: Unknown") + + triggerReconcile() + + // Verify an AgentAction was created and set on the status + require.NotNil(t, ps.Status.Action, "expected Action to be set") + var action porterv1.AgentAction + require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: ps.Namespace, Name: ps.Status.Action.Name}, &action)) + assert.Equal(t, "1", action.Labels[porterv1.LabelResourceGeneration], "The wrong action is set on the status") + + // Mark the action as scheduled + action.Status.Phase = porterv1.PhasePending + action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionScheduled), Status: metav1.ConditionTrue}} + require.NoError(t, controller.Status().Update(ctx, &action)) + + triggerReconcile() + + // Verify the credential set status was synced with the action + assert.Equal(t, porterv1.PhasePending, ps.Status.Phase, "incorrect Phase") + assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionScheduled))) + + // Mark the action as started + action.Status.Phase = porterv1.PhaseRunning + action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionStarted), Status: metav1.ConditionTrue}} + require.NoError(t, controller.Status().Update(ctx, &action)) + + triggerReconcile() + + // Verify the credential set status was synced with the action + assert.Equal(t, porterv1.PhaseRunning, ps.Status.Phase, "incorrect Phase") + assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionStarted))) + + // Complete the action + action.Status.Phase = porterv1.PhaseSucceeded + action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionComplete), Status: metav1.ConditionTrue}} + require.NoError(t, controller.Status().Update(ctx, &action)) + + triggerReconcile() + + // Verify the credential set status was synced with the action + assert.NotNil(t, ps.Status.Action, "expected Action to still be set") + assert.Equal(t, porterv1.PhaseSucceeded, ps.Status.Phase, "incorrect Phase") + assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(string(porterv1.ConditionComplete)))) + + // Fail the action + action.Status.Phase = porterv1.PhaseFailed + action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionFailed), Status: metav1.ConditionTrue}} + require.NoError(t, controller.Status().Update(ctx, &action)) + + triggerReconcile() + + // Verify that the credential set status shows the action is failed + require.NotNil(t, ps.Status.Action, "expected Action to still be set") + assert.Equal(t, porterv1.PhaseFailed, ps.Status.Phase, "incorrect Phase") + assert.True(t, apimeta.IsStatusConditionTrue((ps.Status.Conditions), string(porterv1.ConditionFailed))) + + // Edit the generation spec + ps.Generation = 2 + require.NoError(t, controller.Update(ctx, &ps)) + + triggerReconcile() + + // Verify that the credential set status was re-initialized + assert.Equal(t, int64(2), ps.Status.ObservedGeneration) + assert.Equal(t, porterv1.PhaseUnknown, ps.Status.Phase, "New resources should be initialized to Phase: Unknown") + assert.Empty(t, ps.Status.Conditions, "Conditions should have been reset") + + // Retry the last action + lastAction := ps.Status.Action.Name + ps.Annotations = map[string]string{porterv1.AnnotationRetry: "retry-1"} + require.NoError(t, controller.Update(ctx, &ps)) + + triggerReconcile() + + // Verify that action has retry set on it now + require.NotNil(t, ps.Status.Action, "Expected the action to still be set") + assert.Equal(t, lastAction, ps.Status.Action.Name, "Expected the action to be the same") + // get the latest version of the action + require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: ps.Namespace, Name: ps.Status.Action.Name}, &action)) + assert.NotEmpty(t, action.Annotations[porterv1.AnnotationRetry], "Expected the action to have its retry annotation set") + + assert.Equal(t, int64(2), ps.Status.ObservedGeneration) + assert.NotEmpty(t, ps.Status.Action, "Expected the action to still be set") + assert.Equal(t, porterv1.PhaseUnknown, ps.Status.Phase, "New resources should be initialized to Phase Unknown") + assert.Empty(t, ps.Status.Conditions, "Conditions should have been reset") + + // Delete the credential set (setting the delete timestamp directly instead of client.Delete because otherwise the fake client just removes it immediately) + // The fake client doesn't really follow finalizer logic + now := metav1.NewTime(time.Now()) + ps.Generation = 3 + ps.DeletionTimestamp = &now + require.NoError(t, controller.Update(ctx, &ps)) + + triggerReconcile() + + // Verify that an action was created to delete it + require.NotNil(t, ps.Status.Action, "expected Action to be set") + require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: ps.Namespace, Name: ps.Status.Action.Name}, &action)) + assert.Equal(t, "3", action.Labels[porterv1.LabelResourceGeneration], "The wrong action is set on the status") + + // Complete the delete action + action.Status.Phase = porterv1.PhaseSucceeded + action.Status.Conditions = []metav1.Condition{{Type: string(porterv1.ConditionComplete), Status: metav1.ConditionTrue}} + require.NoError(t, controller.Status().Update(ctx, &action)) + + triggerReconcile() + + // Verify that the credential set was removed + err := controller.Get(ctx, client.ObjectKeyFromObject(&ps), &ps) + require.True(t, apierrors.IsNotFound(err), "expected the credential set was deleted") + + // Verify that the reconcile doesn't error out after its deleted + triggerReconcile() +} + +func TestParameterSetReconciler_createAgentAction(t *testing.T) { + controller := setupParameterSetController() + tests := []struct { + name string + delete bool + }{ + { + name: "Credential Set create agent action", + delete: false, + }, + { + name: "Credential Set delete agent action", + delete: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cs := &porterv1.ParameterSet{ + TypeMeta: metav1.TypeMeta{ + APIVersion: porterv1.GroupVersion.String(), + Kind: "ParameterSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "myCreds", + UID: "random-uid", + Generation: 1, + Labels: map[string]string{ + "testLabel": "abc123", + }, + Annotations: map[string]string{ + porterv1.AnnotationRetry: "2021-2-2 12:00:00", + }, + }, + Spec: porterv1.ParameterSetSpec{ + Namespace: "dev", + Name: "credset", + AgentConfig: &corev1.LocalObjectReference{Name: "myAgentConfig"}, + PorterConfig: &corev1.LocalObjectReference{Name: "myPorterConfig"}, + }, + } + controllerutil.AddFinalizer(cs, porterv1.FinalizerName) + if test.delete { + now := metav1.NewTime(time.Now()) + cs.DeletionTimestamp = &now + } + action, err := controller.createAgentAction(context.Background(), logr.Discard(), cs) + require.NoError(t, err) + assert.Equal(t, "test", action.Namespace) + assert.Contains(t, action.Name, "myCreds-") + assert.Len(t, action.OwnerReferences, 1, "expected an owner reference") + wantOwnerRef := metav1.OwnerReference{ + APIVersion: porterv1.GroupVersion.String(), + Kind: "ParameterSet", + Name: "myCreds", + UID: "random-uid", + Controller: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(true), + } + assert.Equal(t, wantOwnerRef, action.OwnerReferences[0], "incorrect owner reference") + assertContains(t, action.Annotations, porterv1.AnnotationRetry, cs.Annotations[porterv1.AnnotationRetry], "incorrect annotation") + assertContains(t, action.Labels, porterv1.LabelManaged, "true", "incorrect label") + assertContains(t, action.Labels, porterv1.LabelResourceKind, "ParameterSet", "incorrect label") + assertContains(t, action.Labels, porterv1.LabelResourceName, "myCreds", "incorrect label") + assertContains(t, action.Labels, porterv1.LabelResourceGeneration, "1", "incorrect label") + assertContains(t, action.Labels, "testLabel", "abc123", "incorrect label") + + assert.Equal(t, cs.Spec.AgentConfig, action.Spec.AgentConfig, "incorrect AgentConfig reference") + assert.Equal(t, cs.Spec.AgentConfig, action.Spec.AgentConfig, "incorrect PorterConfig reference") + assert.Nilf(t, action.Spec.Command, "should use the default command for the agent") + if test.delete { + assert.Equal(t, []string{"credentials", "delete", "-n", cs.Spec.Namespace, cs.Spec.Name}, action.Spec.Args, "incorrect agent arguments") + assert.Empty(t, action.Spec.Files["credentials.yaml"], "expected credentials.yaml to be empty") + + } else { + assert.Equal(t, []string{"credentials", "apply", "credentials.yaml"}, action.Spec.Args, "incorrect agent arguments") + assert.Contains(t, action.Spec.Files, "credentials.yaml") + assert.NotEmpty(t, action.Spec.Files["credentials.yaml"], "expected credentials.yaml to get set on the action") + credSetYaml, err := cs.Spec.ToPorterDocument() + assert.NoError(t, err) + assert.Equal(t, action.Spec.Files["credentials.yaml"], credSetYaml) + } + + }) + } +} + +func setupParameterSetController(objs ...client.Object) ParameterSetReconciler { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(porterv1.AddToScheme(scheme)) + + fakeBuilder := fake.NewClientBuilder() + fakeBuilder.WithScheme(scheme) + fakeBuilder.WithObjects(objs...) + fakeClient := fakeBuilder.Build() + + return ParameterSetReconciler{ + Log: logr.Discard(), + Client: fakeClient, + Scheme: scheme, + } +} diff --git a/main.go b/main.go index f791a51a..c86b2e3e 100644 --- a/main.go +++ b/main.go @@ -85,6 +85,13 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "CredentialSet") os.Exit(1) } + if err = (&controllers.ParameterSetReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ParameterSet") + os.Exit(1) + } // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { From 0abe748ab3b64c900c7eb3fb7fdffd117d745785 Mon Sep 17 00:00:00 2001 From: Brian DeGeeter Date: Thu, 12 May 2022 16:42:26 -0700 Subject: [PATCH 2/7] basic paramset reconciler with existing tests passing Signed-off-by: Brian DeGeeter --- api/v1/testdata/credential-set.yaml | 6 +- api/v1/zz_generated.deepcopy.go | 51 +++- config/crd/bases/porter.sh_parametersets.yaml | 201 ++++++++++++++ config/crd/kustomization.yaml | 4 +- config/rbac/role.yaml | 26 ++ controllers/parameterset_controller.go | 250 +++++++++++++++++- controllers/parameterset_controller_test.go | 48 ++-- main.go | 1 + 8 files changed, 542 insertions(+), 45 deletions(-) create mode 100644 config/crd/bases/porter.sh_parametersets.yaml diff --git a/api/v1/testdata/credential-set.yaml b/api/v1/testdata/credential-set.yaml index 6b7df3c8..4cbb8a2c 100644 --- a/api/v1/testdata/credential-set.yaml +++ b/api/v1/testdata/credential-set.yaml @@ -2,6 +2,6 @@ schemaVersion: 1.0.1 name: porter-test-me namespace: dev credentials: - - name: insecureValue - source: - secret: test-secret + - name: insecureValue + source: + secret: test-secret diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 7a2e3434..de80d202 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -518,13 +518,29 @@ func (in *OCIReferenceParts) DeepCopy() *OCIReferenceParts { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Parameter) DeepCopyInto(out *Parameter) { + *out = *in + out.Source = in.Source +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Parameter. +func (in *Parameter) DeepCopy() *Parameter { + if in == nil { + return nil + } + out := new(Parameter) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParameterSet) DeepCopyInto(out *ParameterSet) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec - out.Status = in.Status + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSet. @@ -580,6 +596,21 @@ func (in *ParameterSetList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParameterSetSpec) DeepCopyInto(out *ParameterSetSpec) { *out = *in + if in.AgentConfig != nil { + in, out := &in.AgentConfig, &out.AgentConfig + *out = new(corev1.LocalObjectReference) + **out = **in + } + if in.PorterConfig != nil { + in, out := &in.PorterConfig, &out.PorterConfig + *out = new(corev1.LocalObjectReference) + **out = **in + } + if in.Parameters != nil { + in, out := &in.Parameters, &out.Parameters + *out = make([]Parameter, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSetSpec. @@ -595,6 +626,7 @@ func (in *ParameterSetSpec) DeepCopy() *ParameterSetSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParameterSetStatus) DeepCopyInto(out *ParameterSetStatus) { *out = *in + in.PorterResourceStatus.DeepCopyInto(&out.PorterResourceStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSetStatus. @@ -607,6 +639,21 @@ func (in *ParameterSetStatus) DeepCopy() *ParameterSetStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ParameterSource) DeepCopyInto(out *ParameterSource) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParameterSource. +func (in *ParameterSource) DeepCopy() *ParameterSource { + if in == nil { + return nil + } + out := new(ParameterSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PluginConfig) DeepCopyInto(out *PluginConfig) { *out = *in diff --git a/config/crd/bases/porter.sh_parametersets.yaml b/config/crd/bases/porter.sh_parametersets.yaml new file mode 100644 index 00000000..790d63b7 --- /dev/null +++ b/config/crd/bases/porter.sh_parametersets.yaml @@ -0,0 +1,201 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: parametersets.porter.sh +spec: + group: porter.sh + names: + kind: ParameterSet + listKind: ParameterSetList + plural: parametersets + singular: parameterset + scope: Namespaced + versions: + - name: v1 + schema: + openAPIV3Schema: + description: ParameterSet is the Schema for the parametersets API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: ParameterSetSpec defines the desired state of ParameterSet + properties: + agentConfig: + description: AgentConfig is the name of an AgentConfig to use instead + of the AgentConfig defined at the namespace or system level. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + name: + description: Name is the name of the parameter set in Porter. Immutable. + type: string + namespace: + description: Namespace (in Porter) where the parameter set is defined. + type: string + parameters: + description: Parameters list of bundle parameters in the parameter + set. + items: + description: Parameter defines an element in a ParameterSet + properties: + name: + description: Name is the bundle parameter name + type: string + source: + description: 'Source is the bundle parameter source supported: + secret, value unsupported: file path(via configMap), env var, + shell cmd' + properties: + secret: + description: Secret is a parameter source using a secret + plugin + type: string + value: + description: Value is a paremeter source using plaintext + value + type: string + type: object + required: + - name + - source + type: object + type: array + porterConfig: + description: PorterConfig is the name of a PorterConfig to use instead + of the PorterConfig defined at the namespace or system level. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + schemaVersion: + description: SchemaVersion is the version of the parameter set state + schema. + type: string + required: + - name + - namespace + - parameters + - schemaVersion + type: object + status: + description: ParameterSetStatus defines the observed state of ParameterSet + properties: + action: + description: The most recent action executed for the resource + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + conditions: + description: 'Conditions store a list of states that have been reached. + Each condition refers to the status of the ActiveJob Possible conditions + are: Scheduled, Started, Completed, and Failed' + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + type FooStatus struct{ // Represents the observations of a foo's + current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + observedGeneration: + description: The last generation observed by the controller. + format: int64 + type: integer + phase: + description: 'The current status of the agent. Possible values are: + Unknown, Pending, Running, Succeeded, and Failed.' + type: string + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 49901298..934093f8 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -7,7 +7,7 @@ resources: - bases/porter.sh_porterconfigs.yaml - bases/porter.sh_agentactions.yaml - bases/porter.sh_credentialsets.yaml -- bases/porter.sh_parametersets.yaml + - bases/porter.sh_parametersets.yaml # +kubebuilder:scaffold:crdkustomizeresource patchesStrategicMerge: @@ -31,4 +31,4 @@ patchesStrategicMerge: # the following config is for teaching kustomize how to do kustomization for CRDs. configurations: -- kustomizeconfig.yaml + - kustomizeconfig.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 0d11d1be..bd7b0f71 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -132,6 +132,32 @@ rules: - get - patch - update +- apiGroups: + - porter.sh + resources: + - parametersets + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - porter.sh + resources: + - parametersets/finalizers + verbs: + - update +- apiGroups: + - porter.sh + resources: + - parametersets/status + verbs: + - get + - patch + - update - apiGroups: - porter.sh resources: diff --git a/controllers/parameterset_controller.go b/controllers/parameterset_controller.go index 7c4e1d07..a62d6a33 100644 --- a/controllers/parameterset_controller.go +++ b/controllers/parameterset_controller.go @@ -2,18 +2,28 @@ package controllers import ( "context" + "fmt" + "reflect" + "github.com/go-logr/logr" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - portershv1 "get.porter.sh/operator/api/v1" + porterv1 "get.porter.sh/operator/api/v1" ) // ParameterSetReconciler reconciles a ParameterSet object type ParameterSetReconciler struct { client.Client + Log logr.Logger Scheme *runtime.Scheme } @@ -21,26 +31,238 @@ type ParameterSetReconciler struct { //+kubebuilder:rbac:groups=porter.sh,resources=parametersets/status,verbs=get;update;patch //+kubebuilder:rbac:groups=porter.sh,resources=parametersets/finalizers,verbs=update -// Reconcile is part of the main kubernetes reconciliation loop which aims to -// move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the ParameterSet object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.2/pkg/reconcile func (r *ParameterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - _ = log.FromContext(ctx) - // TODO(user): your logic here + log := r.Log.WithValues("parameterSet", req.Name, "namespace", req.Namespace) + ps := &porterv1.ParameterSet{} + err := r.Get(ctx, req.NamespacedName, ps) + if err != nil { + if apierrors.IsNotFound(err) { + log.V(Log5Trace).Info("Reconciliation skipped: ParameterSet CRD or one of its owned resources was deleted.") + return ctrl.Result{}, nil + } + return ctrl.Result{}, err + } + + log = log.WithValues("resourceVersion", ps.ResourceVersion, "generation", ps.Generation) + log.V(Log5Trace).Info("Reconciling parameter set") + + // Check if we have requested an agent run yet + action, handled, err := r.isHandled(ctx, log, ps) + if err != nil { + return ctrl.Result{}, err + } + + if action != nil { + log = log.WithValues("agentaction", action.Name) + } + + if err = r.syncStatus(ctx, log, ps, action); err != nil { + return ctrl.Result{}, err + } + + if isDeleteProcessed(ps) { + err = removeParamSetFinalizer(ctx, log, r.Client, ps) + log.V(Log4Debug).Info("Reconciliation complete: Finalizer has been removed from the ParameterSet.") + return ctrl.Result{}, err + } + + if handled { + // Check if retry was requested + if action.GetRetryLabelValue() != ps.GetRetryLabelValue() { + err = r.retry(ctx, log, ps, action) + log.V(Log4Debug).Info("Reconciliation complete: The associated porter agent action was retried.") + return ctrl.Result{}, err + } + + //Nothing to do + log.V(Log4Debug).Info("Reconciliation complete: A porter agent has already been dispatched.") + return ctrl.Result{}, nil + } + + if r.shouldDelete(ps) { + err = r.runParameterSet(ctx, log, ps) + log.V(Log4Debug).Info("Reconciliation complete: A porter agent has been dispatched to delete the parameter set") + return ctrl.Result{}, err + + } else if isDeleted(ps) { + log.V(Log4Debug).Info("Reconciliation complete: ParameterSet CRD is ready for deletion.") + return ctrl.Result{}, nil + } + + // ensure non-deleted parameter sets have finalizers + updated, err := ensureFinalizerSet(ctx, log, r.Client, ps) + if err != nil { + return ctrl.Result{}, err + } + if updated { + // if we added a finalizer, stop processing and we will finish when the updated resource is reconciled + log.V(Log4Debug).Info("Reconciliation complete: A finalizer has been set on the parameter set.") + return ctrl.Result{}, nil + } + err = r.runParameterSet(ctx, log, ps) + if err != nil { + return ctrl.Result{}, err + } + log.V(Log4Debug).Info("Reconciliation complete: A porter agent has been dispatched to apply changes to the parameter set.") return ctrl.Result{}, nil } // SetupWithManager sets up the controller with the Manager. func (r *ParameterSetReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&portershv1.ParameterSet{}). + For(&porterv1.ParameterSet{}, builder.WithPredicates(resourceChanged{})). + Owns(&porterv1.AgentAction{}). Complete(r) } + +// isHandled determines if this generation of the parameter set resource has been processed by Porter +func (r *ParameterSetReconciler) isHandled(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) (*porterv1.AgentAction, bool, error) { + labels := getActionLabels(ps) + results := porterv1.AgentActionList{} + err := r.List(ctx, &results, client.InNamespace(ps.Namespace), client.MatchingLabels(labels)) + if err != nil { + return nil, false, errors.Wrapf(err, "could not query for the current agent action") + } + + if len(results.Items) == 0 { + log.V(Log4Debug).Info("No existing agent action was found") + return nil, false, nil + } + action := results.Items[0] + log.V(Log4Debug).Info("Found existing agent action", "agentaction", action.Name, "namespace", action.Namespace) + return &action, true, nil +} + +//Check the status of the porter-agent job and use that to update the AgentAction status +func (r *ParameterSetReconciler) syncStatus(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet, action *porterv1.AgentAction) error { + origStatus := ps.Status + + applyAgentAction(log, ps, action) + + if !reflect.DeepEqual(origStatus, ps.Status) { + return r.saveStatus(ctx, log, ps) + } + + return nil +} + +func (r *ParameterSetReconciler) runParameterSet(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) error { + log.V(Log5Trace).Info("Initializing parameter set status") + ps.Status.Initialize() + if err := r.saveStatus(ctx, log, ps); err != nil { + return err + } + + return r.runPorter(ctx, log, ps) +} + +// This could be the main "runFunction for each controller" +// Trigger an agent +func (r *ParameterSetReconciler) runPorter(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) error { + action, err := r.createAgentAction(ctx, log, ps) + if err != nil { + return err + } + + // Update the ParameterSet Status with the agent action + return r.syncStatus(ctx, log, ps, action) +} + +// Only update the status with a PATCH, don't clobber the entire installation +func (r *ParameterSetReconciler) saveStatus(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) error { + log.V(Log5Trace).Info("Patching parameter set status") + return PatchObjectWithRetry(ctx, log, r.Client, r.Client.Status().Patch, ps, func() client.Object { + return &porterv1.ParameterSet{} + }) +} + +// create a porter parameters AgentAction for applying or deleting parameter sets +func (r *ParameterSetReconciler) createAgentAction(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) (*porterv1.AgentAction, error) { + paramSetResourceB, err := ps.Spec.ToPorterDocument() + if err != nil { + return nil, err + } + + action := newPSAgentAction(ps) + log.WithValues("action name", action.Name) + if r.shouldDelete(ps) { + log.V(Log5Trace).Info("Deleting porter parameter set") + action.Spec.Args = []string{"parameters", "delete", "-n", ps.Spec.Namespace, ps.Spec.Name} + } else { + log.V(Log5Trace).Info(fmt.Sprintf("Creating porter parameter set %s", action.Name)) + action.Spec.Args = []string{"parameters", "apply", "parameters.yaml"} + action.Spec.Files = map[string][]byte{"parameters.yaml": paramSetResourceB} + } + + if err := r.Create(ctx, action); err != nil { + return nil, errors.Wrap(err, "error creating the porter parameter set agent action") + } + + log.V(Log4Debug).Info("Created porter parameter set agent action") + return action, nil +} + +func (r *ParameterSetReconciler) shouldDelete(ps *porterv1.ParameterSet) bool { + // ignore a deleted CRD with no finalizers + return isDeleted(ps) && isFinalizerSet(ps) +} + +// Sync the retry annotation from the parameter set to the agent action to trigger another run. +func (r *ParameterSetReconciler) retry(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet, action *porterv1.AgentAction) error { + log.V(Log5Trace).Info("Initializing installation status") + ps.Status.Initialize() + ps.Status.Action = &corev1.LocalObjectReference{Name: action.Name} + if err := r.saveStatus(ctx, log, ps); err != nil { + return err + } + + log.V(Log5Trace).Info("Retrying associated porter agent action") + retry := ps.GetRetryLabelValue() + action.SetRetryAnnotation(retry) + if err := r.Update(ctx, action); err != nil { + return errors.Wrap(err, "error updating the associated porter agent action") + } + + log.V(Log4Debug).Info("Retried associated porter agent action", "name", "retry", action.Name, retry) + return nil +} + +// removeFinalizer deletes the porter finalizer from the specified resource and saves it. +func removeParamSetFinalizer(ctx context.Context, log logr.Logger, client client.Client, ps *porterv1.ParameterSet) error { + log.V(Log5Trace).Info("removing finalizer") + controllerutil.RemoveFinalizer(ps, porterv1.FinalizerName) + return client.Update(ctx, ps) +} + +func newPSAgentAction(ps *porterv1.ParameterSet) *porterv1.AgentAction { + labels := getActionLabels(ps) + for k, v := range ps.Labels { + labels[k] = v + } + + action := &porterv1.AgentAction{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ps.Namespace, + GenerateName: ps.Name + "-", + Labels: labels, + Annotations: ps.Annotations, + OwnerReferences: []metav1.OwnerReference{ + { // I'm not using controllerutil.SetControllerReference because I can't track down why that throws a panic when running our tests + APIVersion: ps.APIVersion, + Kind: ps.Kind, + Name: ps.Name, + UID: ps.UID, + Controller: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(true), + }, + }, + }, + Spec: porterv1.AgentActionSpec{ + AgentConfig: ps.Spec.AgentConfig, + PorterConfig: ps.Spec.PorterConfig, + }, + } + return action +} diff --git a/controllers/parameterset_controller_test.go b/controllers/parameterset_controller_test.go index 89d42af1..90a6e858 100644 --- a/controllers/parameterset_controller_test.go +++ b/controllers/parameterset_controller_test.go @@ -30,7 +30,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { namespace := "test" name := "mybuns" testdata := []client.Object{ - &porterv1.CredentialSet{ + &porterv1.ParameterSet{ ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: name, Generation: 1}, }, } @@ -54,7 +54,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { } triggerReconcile() - // Verify the credential set was picked up and the status initialized + // Verify the parameter set was picked up and the status initialized assert.Equal(t, porterv1.PhaseUnknown, ps.Status.Phase, "New resources should be initialized to Phase: Unknown") triggerReconcile() @@ -72,7 +72,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { triggerReconcile() - // Verify the credential set status was synced with the action + // Verify the parameter set status was synced with the action assert.Equal(t, porterv1.PhasePending, ps.Status.Phase, "incorrect Phase") assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionScheduled))) @@ -83,7 +83,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { triggerReconcile() - // Verify the credential set status was synced with the action + // Verify the parameter set status was synced with the action assert.Equal(t, porterv1.PhaseRunning, ps.Status.Phase, "incorrect Phase") assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionStarted))) @@ -94,7 +94,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { triggerReconcile() - // Verify the credential set status was synced with the action + // Verify the parameter set status was synced with the action assert.NotNil(t, ps.Status.Action, "expected Action to still be set") assert.Equal(t, porterv1.PhaseSucceeded, ps.Status.Phase, "incorrect Phase") assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(string(porterv1.ConditionComplete)))) @@ -106,7 +106,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { triggerReconcile() - // Verify that the credential set status shows the action is failed + // Verify that the parameter set status shows the action is failed require.NotNil(t, ps.Status.Action, "expected Action to still be set") assert.Equal(t, porterv1.PhaseFailed, ps.Status.Phase, "incorrect Phase") assert.True(t, apimeta.IsStatusConditionTrue((ps.Status.Conditions), string(porterv1.ConditionFailed))) @@ -117,7 +117,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { triggerReconcile() - // Verify that the credential set status was re-initialized + // Verify that the parameter set status was re-initialized assert.Equal(t, int64(2), ps.Status.ObservedGeneration) assert.Equal(t, porterv1.PhaseUnknown, ps.Status.Phase, "New resources should be initialized to Phase: Unknown") assert.Empty(t, ps.Status.Conditions, "Conditions should have been reset") @@ -141,7 +141,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { assert.Equal(t, porterv1.PhaseUnknown, ps.Status.Phase, "New resources should be initialized to Phase Unknown") assert.Empty(t, ps.Status.Conditions, "Conditions should have been reset") - // Delete the credential set (setting the delete timestamp directly instead of client.Delete because otherwise the fake client just removes it immediately) + // Delete the parameter set (setting the delete timestamp directly instead of client.Delete because otherwise the fake client just removes it immediately) // The fake client doesn't really follow finalizer logic now := metav1.NewTime(time.Now()) ps.Generation = 3 @@ -162,9 +162,9 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { triggerReconcile() - // Verify that the credential set was removed + // Verify that the parameter set was removed err := controller.Get(ctx, client.ObjectKeyFromObject(&ps), &ps) - require.True(t, apierrors.IsNotFound(err), "expected the credential set was deleted") + require.True(t, apierrors.IsNotFound(err), "expected the parameter set was deleted") // Verify that the reconcile doesn't error out after its deleted triggerReconcile() @@ -177,11 +177,11 @@ func TestParameterSetReconciler_createAgentAction(t *testing.T) { delete bool }{ { - name: "Credential Set create agent action", + name: "Parameter Set create agent action", delete: false, }, { - name: "Credential Set delete agent action", + name: "Parameter Set delete agent action", delete: true, }, } @@ -194,7 +194,7 @@ func TestParameterSetReconciler_createAgentAction(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Namespace: "test", - Name: "myCreds", + Name: "myParams", UID: "random-uid", Generation: 1, Labels: map[string]string{ @@ -206,7 +206,7 @@ func TestParameterSetReconciler_createAgentAction(t *testing.T) { }, Spec: porterv1.ParameterSetSpec{ Namespace: "dev", - Name: "credset", + Name: "paramset", AgentConfig: &corev1.LocalObjectReference{Name: "myAgentConfig"}, PorterConfig: &corev1.LocalObjectReference{Name: "myPorterConfig"}, }, @@ -219,12 +219,12 @@ func TestParameterSetReconciler_createAgentAction(t *testing.T) { action, err := controller.createAgentAction(context.Background(), logr.Discard(), cs) require.NoError(t, err) assert.Equal(t, "test", action.Namespace) - assert.Contains(t, action.Name, "myCreds-") + assert.Contains(t, action.Name, "myParams-") assert.Len(t, action.OwnerReferences, 1, "expected an owner reference") wantOwnerRef := metav1.OwnerReference{ APIVersion: porterv1.GroupVersion.String(), Kind: "ParameterSet", - Name: "myCreds", + Name: "myParams", UID: "random-uid", Controller: pointer.BoolPtr(true), BlockOwnerDeletion: pointer.BoolPtr(true), @@ -233,7 +233,7 @@ func TestParameterSetReconciler_createAgentAction(t *testing.T) { assertContains(t, action.Annotations, porterv1.AnnotationRetry, cs.Annotations[porterv1.AnnotationRetry], "incorrect annotation") assertContains(t, action.Labels, porterv1.LabelManaged, "true", "incorrect label") assertContains(t, action.Labels, porterv1.LabelResourceKind, "ParameterSet", "incorrect label") - assertContains(t, action.Labels, porterv1.LabelResourceName, "myCreds", "incorrect label") + assertContains(t, action.Labels, porterv1.LabelResourceName, "myParams", "incorrect label") assertContains(t, action.Labels, porterv1.LabelResourceGeneration, "1", "incorrect label") assertContains(t, action.Labels, "testLabel", "abc123", "incorrect label") @@ -241,16 +241,16 @@ func TestParameterSetReconciler_createAgentAction(t *testing.T) { assert.Equal(t, cs.Spec.AgentConfig, action.Spec.AgentConfig, "incorrect PorterConfig reference") assert.Nilf(t, action.Spec.Command, "should use the default command for the agent") if test.delete { - assert.Equal(t, []string{"credentials", "delete", "-n", cs.Spec.Namespace, cs.Spec.Name}, action.Spec.Args, "incorrect agent arguments") - assert.Empty(t, action.Spec.Files["credentials.yaml"], "expected credentials.yaml to be empty") + assert.Equal(t, []string{"parameters", "delete", "-n", cs.Spec.Namespace, cs.Spec.Name}, action.Spec.Args, "incorrect agent arguments") + assert.Empty(t, action.Spec.Files["parameters.yaml"], "expected parameters.yaml to be empty") } else { - assert.Equal(t, []string{"credentials", "apply", "credentials.yaml"}, action.Spec.Args, "incorrect agent arguments") - assert.Contains(t, action.Spec.Files, "credentials.yaml") - assert.NotEmpty(t, action.Spec.Files["credentials.yaml"], "expected credentials.yaml to get set on the action") - credSetYaml, err := cs.Spec.ToPorterDocument() + assert.Equal(t, []string{"parameters", "apply", "parameters.yaml"}, action.Spec.Args, "incorrect agent arguments") + assert.Contains(t, action.Spec.Files, "parameters.yaml") + assert.NotEmpty(t, action.Spec.Files["parameters.yaml"], "expected parameters.yaml to get set on the action") + paramSetYaml, err := cs.Spec.ToPorterDocument() assert.NoError(t, err) - assert.Equal(t, action.Spec.Files["credentials.yaml"], credSetYaml) + assert.Equal(t, action.Spec.Files["parameters.yaml"], paramSetYaml) } }) diff --git a/main.go b/main.go index c86b2e3e..c0956d21 100644 --- a/main.go +++ b/main.go @@ -87,6 +87,7 @@ func main() { } if err = (&controllers.ParameterSetReconciler{ Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("ParameterSet"), Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ParameterSet") From b5b9fcaa65659b6d5314ba8d0e38541dfff2ad62 Mon Sep 17 00:00:00 2001 From: Brian DeGeeter Date: Thu, 12 May 2022 18:11:08 -0700 Subject: [PATCH 3/7] initial passing paramset integration test Signed-off-by: Brian DeGeeter --- controllers/parameterset_controller.go | 21 +- tests/integration/paramset_test.go | 403 +++++++++++++++++++++++++ tests/integration/suite_test.go | 6 + 3 files changed, 422 insertions(+), 8 deletions(-) create mode 100644 tests/integration/paramset_test.go diff --git a/controllers/parameterset_controller.go b/controllers/parameterset_controller.go index a62d6a33..7cef801e 100644 --- a/controllers/parameterset_controller.go +++ b/controllers/parameterset_controller.go @@ -30,6 +30,19 @@ type ParameterSetReconciler struct { //+kubebuilder:rbac:groups=porter.sh,resources=parametersets,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=porter.sh,resources=parametersets/status,verbs=get;update;patch //+kubebuilder:rbac:groups=porter.sh,resources=parametersets/finalizers,verbs=update +//+kubebuilder:rbac:groups=porter.sh,resources=agentconfigs,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=porter.sh,resources=porterconfigs,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete + +// SetupWithManager sets up the controller with the Manager. +func (r *ParameterSetReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&porterv1.ParameterSet{}, builder.WithPredicates(resourceChanged{})). + Owns(&porterv1.AgentAction{}). + Complete(r) +} func (r *ParameterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -109,14 +122,6 @@ func (r *ParameterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, nil } -// SetupWithManager sets up the controller with the Manager. -func (r *ParameterSetReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&porterv1.ParameterSet{}, builder.WithPredicates(resourceChanged{})). - Owns(&porterv1.AgentAction{}). - Complete(r) -} - // isHandled determines if this generation of the parameter set resource has been processed by Porter func (r *ParameterSetReconciler) isHandled(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) (*porterv1.AgentAction, bool, error) { labels := getActionLabels(ps) diff --git a/tests/integration/paramset_test.go b/tests/integration/paramset_test.go new file mode 100644 index 00000000..4f7f275b --- /dev/null +++ b/tests/integration/paramset_test.go @@ -0,0 +1,403 @@ +//go:build integration + +package integration_test + +import ( + "context" + "fmt" + "time" + + "get.porter.sh/operator/controllers" + "github.com/carolynvs/magex/shx" + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo" + "github.com/pkg/errors" + "github.com/tidwall/gjson" + batchv1 "k8s.io/api/batch/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apitypes "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + porterv1 "get.porter.sh/operator/api/v1" + . "github.com/onsi/gomega" +) + +/* +var _ = Describe("CredentialSet secret does not exist", func() { + Context("when an installation is created with a CredentialSet resource that references a secret that does not exist", func() { + It("should fail the installation install", func() { + ctx := context.Background() + ns := createTestNamespace(ctx) + name := "test-cs-" + ns + By("successfully creating the credential set", func() { + Log(fmt.Sprintf("create credential set '%s' for credset", name)) + cs := NewTestCredSet(name) + cs.ObjectMeta.Namespace = ns + cred := porterv1.Credential{ + Name: "insecureValue", + Source: porterv1.CredentialSource{ + Secret: name, + }, + } + cs.Spec.Credentials = append(cs.Spec.Credentials, cred) + cs.Spec.Namespace = ns + + Expect(k8sClient.Create(ctx, cs)).Should(Succeed()) + Expect(waitForPorterCS(ctx, cs, "waiting for credential set to apply")).Should(Succeed()) + validateCredSetConditions(cs) + + }) + By("failing the installation install", func() { + Log("install porter-test-me bundle with new credset") + inst := NewTestInstallation("cs-no-secret") + inst.ObjectMeta.Namespace = ns + inst.Spec.Namespace = ns + inst.Spec.CredentialSets = append(inst.Spec.CredentialSets, name) + inst.Spec.SchemaVersion = "1.0.0" + Expect(k8sClient.Create(ctx, inst)).Should(Succeed()) + err := waitForPorter(ctx, inst, "waiting for porter-test-me to install") + Expect(err).Should(HaveOccurred()) + validateInstallationConditions(inst) + Expect(inst.Status.Phase).To(Equal(porterv1.PhaseFailed)) + }) + }) + }) +}) +*/ + +// TODO: Add failed installation due to missing secret test +var _ = FDescribe("ParameterSet lifecycle", func() { + Context("TBD", func() { + It("should run porter apply", func() { + By("creating an agent action", func() { + ctx := context.Background() + ns := createTestNamespace(ctx) + testSecret := "param-test" + name := "ps-update-" + ns + createTestSecret(ctx, name, testSecret, ns) + + Log(fmt.Sprintf("create parameter set '%s'", name)) + ps := NewTestParamSet(name) + ps.Spec.Name = "ps-lifecyce-test" + ps.ObjectMeta.Namespace = ns + param := porterv1.Parameter{ + Name: "test-parameter", + Source: porterv1.ParameterSource{ + Secret: name, + }, + } + ps.Spec.Parameters = append(ps.Spec.Parameters, param) + ps.Spec.Namespace = ns + + Expect(k8sClient.Create(ctx, ps)).Should(Succeed()) + Expect(waitForPorterPS(ctx, ps, "waiting for parameter set to apply")).Should(Succeed()) + validateParamSetConditions(ps) + + Log("verify it's created") + jsonOut := runAgentAction(ctx, "create-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + firstName := gjson.Get(jsonOut, "0.name").String() + numParamSets := gjson.Get(jsonOut, "#").Int() + numParams := gjson.Get(jsonOut, "0.parameters.#").Int() + firstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() + Expect(int64(1)).To(Equal(numParamSets)) + Expect(int64(1)).To(Equal(numParams)) + Expect(ps.Spec.Name).To(Equal(firstName)) + Expect("test-parameter").To(Equal(firstParamName)) + + Log("update a parameter set") + newSecret := "update-paramset" + newParam := porterv1.Parameter{ + Name: "new-parameter", + Source: porterv1.ParameterSource{ + Secret: newSecret, + }, + } + ps.Spec.Parameters = append(ps.Spec.Parameters, newParam) + patchPS := func(ps *porterv1.ParameterSet) { + controllers.PatchObjectWithRetry(ctx, logr.Discard(), k8sClient, k8sClient.Patch, ps, func() client.Object { + return &porterv1.ParameterSet{} + }) + } + patchPS(ps) + Expect(waitForPorterPS(ctx, ps, "waiting for parameters update to apply")).Should(Succeed()) + Log("verify it's updated") + jsonOut = runAgentAction(ctx, "update-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + updatedFirstName := gjson.Get(jsonOut, "0.name").String() + updatedNumParamSets := gjson.Get(jsonOut, "#").Int() + updatedNumParams := gjson.Get(jsonOut, "0.parameters.#").Int() + updatedFirstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() + updatedSecondParamName := gjson.Get(jsonOut, "0.parameters.1.name").String() + Expect(int64(1)).To(Equal(updatedNumParamSets)) + Expect(int64(2)).To(Equal(updatedNumParams)) + Expect(ps.Spec.Name).To(Equal(updatedFirstName)) + Expect("test-parameter").To(Equal(updatedFirstParamName)) + Expect("new-parameter").To(Equal(updatedSecondParamName)) + }) + }) + }) +}) + +var _ = Describe("ParameterSet delete", func() { + Context("when an existing ParameterSet is delete", func() { + It("should run porter parameters delete", func() { + By("creating an agent action", func() { + ctx := context.Background() + ns := createTestNamespace(ctx) + name := "ps-delete-" + ns + testSecret := "secret-value" + + Log(fmt.Sprintf("create k8s secret '%s' for paramset", name)) + psSecret := NewTestSecret(name, testSecret) + psSecret.ObjectMeta.Namespace = ns + Expect(k8sClient.Create(ctx, psSecret)).Should(Succeed()) + + Log(fmt.Sprintf("create parameter set '%s'", name)) + ps := NewTestParamSet(name) + ps.Spec.Name = "ps-delete-test" + ps.ObjectMeta.Namespace = ns + param := porterv1.Parameter{ + Name: "test-parameter", + Source: porterv1.ParameterSource{ + Secret: name, + }, + } + ps.Spec.Parameters = append(ps.Spec.Parameters, param) + ps.Spec.Namespace = ns + + Expect(k8sClient.Create(ctx, ps)).Should(Succeed()) + Expect(waitForPorterPS(ctx, ps, "waiting for parameter set to apply")).Should(Succeed()) + validateParamSetConditions(ps) + + Log("verify it's created") + jsonOut := runAgentAction(ctx, "create-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + firstName := gjson.Get(jsonOut, "0.name").String() + numParams := gjson.Get(jsonOut, "#").Int() + firstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() + Expect(int64(1)).To(Equal(numParams)) + Expect(ps.Spec.Name).To(Equal(firstName)) + Expect("test-parameter").To(Equal(firstParamName)) + + Log("delete a parameter set") + Expect(k8sClient.Delete(ctx, ps)).Should(Succeed()) + Expect(waitForParamSetDeleted(ctx, ps)).Should(Succeed()) + + Log("verify parameter set is gone from porter data store") + delJsonOut := runAgentAction(ctx, "delete-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + delNumParams := gjson.Get(delJsonOut, "#").Int() + Expect(int64(0)).To(Equal(delNumParams)) + + Log("verify parameter set CRD is gone from k8s cluster") + nsName := apitypes.NamespacedName{Namespace: ps.Namespace, Name: ps.Name} + getPS := &porterv1.ParameterSet{} + Expect(k8sClient.Get(ctx, nsName, getPS)).ShouldNot(Succeed()) + }) + }) + }) +}) + +//NewTestParamSet minimal ParameterSet CRD for tests +func NewTestParamSet(psName string) *porterv1.ParameterSet { + ps := &porterv1.ParameterSet{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "porter.sh/v1", + Kind: "ParameterSet", + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "porter-test-me-", + }, + Spec: porterv1.ParameterSetSpec{ + //TODO: get schema version from porter version? + // https://github.com/getporter/porter/pull/2052 + SchemaVersion: "1.0.1", + Name: psName, + }, + } + return ps +} + +/* +func createTestSecret(ctx context.Context, name, value, namespace string) { + Log(fmt.Sprintf("create k8s secret '%s' for credset", name)) + csSecret := NewTestSecret(name, value) + csSecret.ObjectMeta.Namespace = namespace + Expect(k8sClient.Create(ctx, csSecret)).Should(Succeed()) +} + +func NewTestSecret(name, value string) *corev1.Secret { + csSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Type: corev1.SecretTypeOpaque, + StringData: map[string]string{"value": value}, + } + return csSecret +} + +func NewTestInstallation(iName string) *porterv1.Installation { + inst := &porterv1.Installation{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "porter.sh/v1", + Kind: "Installation", + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "porter-test-me-", + }, + Spec: porterv1.InstallationSpec{ + SchemaVersion: "1.0.1", + Name: iName, + Bundle: porterv1.OCIReferenceParts{ + Repository: "ghcr.io/bdegeeter/porter-test-me", + Version: "0.3.0", + }, + }, + } + return inst +} + +func newAgentAction(namespace string, name string, cmd []string) *porterv1.AgentAction { + return &porterv1.AgentAction{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + Spec: porterv1.AgentActionSpec{ + Args: cmd, + }, + } +} + +func waitForAgentAction(ctx context.Context, aa *porterv1.AgentAction, msg string) error { + Log("%s: %s/%s", msg, aa.Namespace, aa.Name) + key := client.ObjectKey{Namespace: aa.Namespace, Name: aa.Name} + ctx, cancel := context.WithTimeout(ctx, getWaitTimeout()) + defer cancel() + for { + select { + case <-ctx.Done(): + Fail(errors.Wrapf(ctx.Err(), "timeout %s", msg).Error()) + default: + err := k8sClient.Get(ctx, key, aa) + if err != nil { + // There is lag between creating and being able to retrieve, I don't understand why + if apierrors.IsNotFound(err) { + time.Sleep(time.Second) + continue + } + return err + } + + // Check if the latest change has been processed + if aa.Generation == aa.Status.ObservedGeneration { + if apimeta.IsStatusConditionTrue(aa.Status.Conditions, string(porterv1.ConditionComplete)) { + return nil + } + + if apimeta.IsStatusConditionTrue(aa.Status.Conditions, string(porterv1.ConditionFailed)) { + // Grab some extra info to help with debugging + //debugFailedCSCreate(ctx, aa) + return errors.New("porter did not run successfully") + } + } + + time.Sleep(time.Second) + continue + } + } + +} +*/ + +func waitForPorterPS(ctx context.Context, ps *porterv1.ParameterSet, msg string) error { + Log("%s: %s/%s", msg, ps.Namespace, ps.Name) + key := client.ObjectKey{Namespace: ps.Namespace, Name: ps.Name} + ctx, cancel := context.WithTimeout(ctx, getWaitTimeout()) + defer cancel() + for { + select { + case <-ctx.Done(): + Fail(errors.Wrapf(ctx.Err(), "timeout %s", msg).Error()) + default: + err := k8sClient.Get(ctx, key, ps) + if err != nil { + // There is lag between creating and being able to retrieve, I don't understand why + if apierrors.IsNotFound(err) { + time.Sleep(time.Second) + continue + } + return err + } + + // Check if the latest change has been processed + if ps.Generation == ps.Status.ObservedGeneration { + if apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionComplete)) { + time.Sleep(time.Second) + return nil + } + + if apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionFailed)) { + // Grab some extra info to help with debugging + debugFailedPSCreate(ctx, ps) + return errors.New("porter did not run successfully") + } + } + + time.Sleep(time.Second) + continue + } + } +} + +func waitForParamSetDeleted(ctx context.Context, ps *porterv1.ParameterSet) error { + Log("Waiting for ParameterSet to finish deleting: %s/%s", ps.Namespace, ps.Name) + key := client.ObjectKey{Namespace: ps.Namespace, Name: ps.Name} + waitCtx, cancel := context.WithTimeout(ctx, getWaitTimeout()) + defer cancel() + for { + select { + case <-waitCtx.Done(): + Fail(errors.Wrap(waitCtx.Err(), "timeout waiting for ParameterSet to delete").Error()) + default: + err := k8sClient.Get(ctx, key, ps) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + time.Sleep(time.Second) + continue + } + } +} + +func debugFailedPSCreate(ctx context.Context, ps *porterv1.ParameterSet) { + Log("DEBUG: ----------------------------------------------------") + actionKey := client.ObjectKey{Name: ps.Status.Action.Name, Namespace: ps.Namespace} + action := &porterv1.AgentAction{} + if err := k8sClient.Get(ctx, actionKey, action); err != nil { + Log(errors.Wrap(err, "could not retrieve the ParameterSet's AgentAction to troubleshoot").Error()) + return + } + + jobKey := client.ObjectKey{Name: action.Status.Job.Name, Namespace: action.Namespace} + job := &batchv1.Job{} + if err := k8sClient.Get(ctx, jobKey, job); err != nil { + Log(errors.Wrap(err, "could not retrieve the ParameterSet's Job to troubleshoot").Error()) + return + } + + shx.Command("kubectl", "logs", "-n="+job.Namespace, "job/"+job.Name). + Env("KUBECONFIG=" + "../../kind.config").RunV() + Log("DEBUG: ----------------------------------------------------") +} + +func validateParamSetConditions(ps *porterv1.ParameterSet) { + // Checks that all expected conditions are set + Expect(apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionScheduled))) + Expect(apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionStarted))) + Expect(apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionComplete))) +} diff --git a/tests/integration/suite_test.go b/tests/integration/suite_test.go index 457ed406..908bc2d6 100644 --- a/tests/integration/suite_test.go +++ b/tests/integration/suite_test.go @@ -91,6 +91,12 @@ var _ = BeforeSuite(func(done Done) { }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) + err = (&controllers.ParameterSetReconciler{ + Client: k8sManager.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("ParameterSet"), + }).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + err = (&controllers.AgentActionReconciler{ Client: k8sManager.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("AgentAction"), From 9d1f40fd454eaa409830f33bac0e74ce6ca015d5 Mon Sep 17 00:00:00 2001 From: Brian DeGeeter Date: Tue, 17 May 2022 15:57:11 -0700 Subject: [PATCH 4/7] add paramset installation test Signed-off-by: Brian DeGeeter --- Makefile | 3 + magefiles/magefile.go | 2 +- tests/integration/paramset_test.go | 356 ++++++++--------------------- 3 files changed, 100 insertions(+), 261 deletions(-) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..85201921 --- /dev/null +++ b/Makefile @@ -0,0 +1,3 @@ +# target for controller-gen +generate: + mage -v GenerateController \ No newline at end of file diff --git a/magefiles/magefile.go b/magefiles/magefile.go index c1518f29..4c611621 100644 --- a/magefiles/magefile.go +++ b/magefiles/magefile.go @@ -505,7 +505,7 @@ func CleanTestdata() { continue } - output, _ = kubectl("get", "installation,credentialset,agentaction", "-n", namespace, `--template={{range .items}}{{.kind}}/{{.metadata.name}},{{end}}`). + output, _ = kubectl("get", "installation,credentialset,parameterset,agentaction", "-n", namespace, `--template={{range .items}}{{.kind}}/{{.metadata.name}},{{end}}`). Output() resources := strings.Split(output, ",") for _, resource := range resources { diff --git a/tests/integration/paramset_test.go b/tests/integration/paramset_test.go index 4f7f275b..cdd00ccb 100644 --- a/tests/integration/paramset_test.go +++ b/tests/integration/paramset_test.go @@ -24,175 +24,104 @@ import ( . "github.com/onsi/gomega" ) -/* -var _ = Describe("CredentialSet secret does not exist", func() { - Context("when an installation is created with a CredentialSet resource that references a secret that does not exist", func() { - It("should fail the installation install", func() { +var _ = Describe("ParameterSet lifecycle", func() { + It("should run porter apply", func() { + By("creating an agent action", func() { ctx := context.Background() ns := createTestNamespace(ctx) - name := "test-cs-" + ns - By("successfully creating the credential set", func() { - Log(fmt.Sprintf("create credential set '%s' for credset", name)) - cs := NewTestCredSet(name) - cs.ObjectMeta.Namespace = ns - cred := porterv1.Credential{ - Name: "insecureValue", - Source: porterv1.CredentialSource{ - Secret: name, - }, - } - cs.Spec.Credentials = append(cs.Spec.Credentials, cred) - cs.Spec.Namespace = ns - - Expect(k8sClient.Create(ctx, cs)).Should(Succeed()) - Expect(waitForPorterCS(ctx, cs, "waiting for credential set to apply")).Should(Succeed()) - validateCredSetConditions(cs) - - }) - By("failing the installation install", func() { - Log("install porter-test-me bundle with new credset") - inst := NewTestInstallation("cs-no-secret") - inst.ObjectMeta.Namespace = ns - inst.Spec.Namespace = ns - inst.Spec.CredentialSets = append(inst.Spec.CredentialSets, name) - inst.Spec.SchemaVersion = "1.0.0" - Expect(k8sClient.Create(ctx, inst)).Should(Succeed()) - err := waitForPorter(ctx, inst, "waiting for porter-test-me to install") - Expect(err).Should(HaveOccurred()) - validateInstallationConditions(inst) - Expect(inst.Status.Phase).To(Equal(porterv1.PhaseFailed)) - }) - }) - }) -}) -*/ - -// TODO: Add failed installation due to missing secret test -var _ = FDescribe("ParameterSet lifecycle", func() { - Context("TBD", func() { - It("should run porter apply", func() { - By("creating an agent action", func() { - ctx := context.Background() - ns := createTestNamespace(ctx) - testSecret := "param-test" - name := "ps-update-" + ns - createTestSecret(ctx, name, testSecret, ns) - - Log(fmt.Sprintf("create parameter set '%s'", name)) - ps := NewTestParamSet(name) - ps.Spec.Name = "ps-lifecyce-test" - ps.ObjectMeta.Namespace = ns - param := porterv1.Parameter{ - Name: "test-parameter", - Source: porterv1.ParameterSource{ - Secret: name, - }, - } - ps.Spec.Parameters = append(ps.Spec.Parameters, param) - ps.Spec.Namespace = ns - - Expect(k8sClient.Create(ctx, ps)).Should(Succeed()) - Expect(waitForPorterPS(ctx, ps, "waiting for parameter set to apply")).Should(Succeed()) - validateParamSetConditions(ps) - - Log("verify it's created") - jsonOut := runAgentAction(ctx, "create-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) - firstName := gjson.Get(jsonOut, "0.name").String() - numParamSets := gjson.Get(jsonOut, "#").Int() - numParams := gjson.Get(jsonOut, "0.parameters.#").Int() - firstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() - Expect(int64(1)).To(Equal(numParamSets)) - Expect(int64(1)).To(Equal(numParams)) - Expect(ps.Spec.Name).To(Equal(firstName)) - Expect("test-parameter").To(Equal(firstParamName)) - - Log("update a parameter set") - newSecret := "update-paramset" - newParam := porterv1.Parameter{ - Name: "new-parameter", - Source: porterv1.ParameterSource{ - Secret: newSecret, - }, - } - ps.Spec.Parameters = append(ps.Spec.Parameters, newParam) - patchPS := func(ps *porterv1.ParameterSet) { - controllers.PatchObjectWithRetry(ctx, logr.Discard(), k8sClient, k8sClient.Patch, ps, func() client.Object { - return &porterv1.ParameterSet{} - }) - } - patchPS(ps) - Expect(waitForPorterPS(ctx, ps, "waiting for parameters update to apply")).Should(Succeed()) - Log("verify it's updated") - jsonOut = runAgentAction(ctx, "update-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) - updatedFirstName := gjson.Get(jsonOut, "0.name").String() - updatedNumParamSets := gjson.Get(jsonOut, "#").Int() - updatedNumParams := gjson.Get(jsonOut, "0.parameters.#").Int() - updatedFirstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() - updatedSecondParamName := gjson.Get(jsonOut, "0.parameters.1.name").String() - Expect(int64(1)).To(Equal(updatedNumParamSets)) - Expect(int64(2)).To(Equal(updatedNumParams)) - Expect(ps.Spec.Name).To(Equal(updatedFirstName)) - Expect("test-parameter").To(Equal(updatedFirstParamName)) - Expect("new-parameter").To(Equal(updatedSecondParamName)) - }) - }) - }) -}) - -var _ = Describe("ParameterSet delete", func() { - Context("when an existing ParameterSet is delete", func() { - It("should run porter parameters delete", func() { - By("creating an agent action", func() { - ctx := context.Background() - ns := createTestNamespace(ctx) - name := "ps-delete-" + ns - testSecret := "secret-value" - - Log(fmt.Sprintf("create k8s secret '%s' for paramset", name)) - psSecret := NewTestSecret(name, testSecret) - psSecret.ObjectMeta.Namespace = ns - Expect(k8sClient.Create(ctx, psSecret)).Should(Succeed()) - - Log(fmt.Sprintf("create parameter set '%s'", name)) - ps := NewTestParamSet(name) - ps.Spec.Name = "ps-delete-test" - ps.ObjectMeta.Namespace = ns - param := porterv1.Parameter{ - Name: "test-parameter", - Source: porterv1.ParameterSource{ - Secret: name, - }, - } - ps.Spec.Parameters = append(ps.Spec.Parameters, param) - ps.Spec.Namespace = ns - - Expect(k8sClient.Create(ctx, ps)).Should(Succeed()) - Expect(waitForPorterPS(ctx, ps, "waiting for parameter set to apply")).Should(Succeed()) - validateParamSetConditions(ps) - - Log("verify it's created") - jsonOut := runAgentAction(ctx, "create-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) - firstName := gjson.Get(jsonOut, "0.name").String() - numParams := gjson.Get(jsonOut, "#").Int() - firstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() - Expect(int64(1)).To(Equal(numParams)) - Expect(ps.Spec.Name).To(Equal(firstName)) - Expect("test-parameter").To(Equal(firstParamName)) - - Log("delete a parameter set") - Expect(k8sClient.Delete(ctx, ps)).Should(Succeed()) - Expect(waitForParamSetDeleted(ctx, ps)).Should(Succeed()) - - Log("verify parameter set is gone from porter data store") - delJsonOut := runAgentAction(ctx, "delete-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) - delNumParams := gjson.Get(delJsonOut, "#").Int() - Expect(int64(0)).To(Equal(delNumParams)) + delayValue := "1" + resourceName := "ps-lifecycle-" + ns + pSetName := "ps-lifecycle-test" + createTestSecret(ctx, resourceName, delayValue, ns) + + Log(fmt.Sprintf("create parameter set '%s'", resourceName)) + paramName := "delay" + ps := NewTestParamSet(resourceName) + ps.Spec.Name = pSetName + ps.ObjectMeta.Namespace = ns + param := porterv1.Parameter{ + Name: paramName, + Source: porterv1.ParameterSource{ + Secret: resourceName, + }, + } + ps.Spec.Parameters = append(ps.Spec.Parameters, param) + ps.Spec.Namespace = ns + + Expect(k8sClient.Create(ctx, ps)).Should(Succeed()) + Expect(waitForPorterPS(ctx, ps, "waiting for parameter set to apply")).Should(Succeed()) + validateParamSetConditions(ps) + + Log("verify it's created") + jsonOut := runAgentAction(ctx, "create-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + firstName := gjson.Get(jsonOut, "0.name").String() + numParamSets := gjson.Get(jsonOut, "#").Int() + numParams := gjson.Get(jsonOut, "0.parameters.#").Int() + firstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() + Expect(int64(1)).To(Equal(numParamSets)) + Expect(int64(1)).To(Equal(numParams)) + Expect(ps.Spec.Name).To(Equal(firstName)) + Expect(paramName).To(Equal(firstParamName)) + + Log("install porter-test-me bundle with new param set") + inst := NewTestInstallation("ps-with-secret") + inst.ObjectMeta.Namespace = ns + inst.Spec.Namespace = ns + inst.Spec.ParameterSets = append(inst.Spec.ParameterSets, pSetName) + inst.Spec.SchemaVersion = "1.0.0" + Expect(k8sClient.Create(ctx, inst)).Should(Succeed()) + Expect(waitForPorter(ctx, inst, "waiting for porter-test-me to install")).Should(Succeed()) + validateInstallationConditions(inst) + + // Validate that the correct parameter set was used by the installation + instJsonOut := runAgentAction(ctx, "show-outputs", ns, []string{"installation", "outputs", "list", "-n", ns, "-i", inst.Spec.Name, "-o", "json"}) + outDelayValue := gjson.Get(instJsonOut, `#(name=="outDelay").value`).String() + Expect(outDelayValue).To(Equal(delayValue)) + + Log("update a parameter set") + updateParamName := "update-paramset" + updateParamValue := "update-value" + newParam := porterv1.Parameter{ + Name: updateParamName, + Source: porterv1.ParameterSource{ + Value: updateParamValue, + }, + } + ps.Spec.Parameters = append(ps.Spec.Parameters, newParam) + patchPS := func(ps *porterv1.ParameterSet) { + controllers.PatchObjectWithRetry(ctx, logr.Discard(), k8sClient, k8sClient.Patch, ps, func() client.Object { + return &porterv1.ParameterSet{} + }) + } + patchPS(ps) + Expect(waitForPorterPS(ctx, ps, "waiting for parameters update to apply")).Should(Succeed()) + Log("verify it's updated") + jsonOut = runAgentAction(ctx, "update-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + updatedFirstName := gjson.Get(jsonOut, "0.name").String() + updatedNumParamSets := gjson.Get(jsonOut, "#").Int() + updatedNumParams := gjson.Get(jsonOut, "0.parameters.#").Int() + updatedFirstParamName := gjson.Get(jsonOut, "0.parameters.0.name").String() + updatedSecondParamName := gjson.Get(jsonOut, "0.parameters.1.name").String() + Expect(int64(1)).To(Equal(updatedNumParamSets)) + Expect(int64(2)).To(Equal(updatedNumParams)) + Expect(ps.Spec.Name).To(Equal(updatedFirstName)) + Expect(paramName).To(Equal(updatedFirstParamName)) + Expect(updateParamName).To(Equal(updatedSecondParamName)) + + Log("delete a parameter set") + Expect(k8sClient.Delete(ctx, ps)).Should(Succeed()) + Expect(waitForParamSetDeleted(ctx, ps)).Should(Succeed()) + + Log("verify parameter set is gone from porter data store") + delJsonOut := runAgentAction(ctx, "delete-check-parameters-list", ns, []string{"parameters", "list", "-n", ns, "-o", "json"}) + delNumParams := gjson.Get(delJsonOut, "#").Int() + Expect(int64(0)).To(Equal(delNumParams)) + + Log("verify parameter set CRD is gone from k8s cluster") + nsName := apitypes.NamespacedName{Namespace: ps.Namespace, Name: ps.Name} + getPS := &porterv1.ParameterSet{} + Expect(k8sClient.Get(ctx, nsName, getPS)).ShouldNot(Succeed()) - Log("verify parameter set CRD is gone from k8s cluster") - nsName := apitypes.NamespacedName{Namespace: ps.Namespace, Name: ps.Name} - getPS := &porterv1.ParameterSet{} - Expect(k8sClient.Get(ctx, nsName, getPS)).ShouldNot(Succeed()) - }) }) }) }) @@ -208,7 +137,7 @@ func NewTestParamSet(psName string) *porterv1.ParameterSet { GenerateName: "porter-test-me-", }, Spec: porterv1.ParameterSetSpec{ - //TODO: get schema version from porter version? + //TODO: get schema version from porter version // https://github.com/getporter/porter/pull/2052 SchemaVersion: "1.0.1", Name: psName, @@ -217,99 +146,6 @@ func NewTestParamSet(psName string) *porterv1.ParameterSet { return ps } -/* -func createTestSecret(ctx context.Context, name, value, namespace string) { - Log(fmt.Sprintf("create k8s secret '%s' for credset", name)) - csSecret := NewTestSecret(name, value) - csSecret.ObjectMeta.Namespace = namespace - Expect(k8sClient.Create(ctx, csSecret)).Should(Succeed()) -} - -func NewTestSecret(name, value string) *corev1.Secret { - csSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Type: corev1.SecretTypeOpaque, - StringData: map[string]string{"value": value}, - } - return csSecret -} - -func NewTestInstallation(iName string) *porterv1.Installation { - inst := &porterv1.Installation{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "porter.sh/v1", - Kind: "Installation", - }, - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "porter-test-me-", - }, - Spec: porterv1.InstallationSpec{ - SchemaVersion: "1.0.1", - Name: iName, - Bundle: porterv1.OCIReferenceParts{ - Repository: "ghcr.io/bdegeeter/porter-test-me", - Version: "0.3.0", - }, - }, - } - return inst -} - -func newAgentAction(namespace string, name string, cmd []string) *porterv1.AgentAction { - return &porterv1.AgentAction{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Spec: porterv1.AgentActionSpec{ - Args: cmd, - }, - } -} - -func waitForAgentAction(ctx context.Context, aa *porterv1.AgentAction, msg string) error { - Log("%s: %s/%s", msg, aa.Namespace, aa.Name) - key := client.ObjectKey{Namespace: aa.Namespace, Name: aa.Name} - ctx, cancel := context.WithTimeout(ctx, getWaitTimeout()) - defer cancel() - for { - select { - case <-ctx.Done(): - Fail(errors.Wrapf(ctx.Err(), "timeout %s", msg).Error()) - default: - err := k8sClient.Get(ctx, key, aa) - if err != nil { - // There is lag between creating and being able to retrieve, I don't understand why - if apierrors.IsNotFound(err) { - time.Sleep(time.Second) - continue - } - return err - } - - // Check if the latest change has been processed - if aa.Generation == aa.Status.ObservedGeneration { - if apimeta.IsStatusConditionTrue(aa.Status.Conditions, string(porterv1.ConditionComplete)) { - return nil - } - - if apimeta.IsStatusConditionTrue(aa.Status.Conditions, string(porterv1.ConditionFailed)) { - // Grab some extra info to help with debugging - //debugFailedCSCreate(ctx, aa) - return errors.New("porter did not run successfully") - } - } - - time.Sleep(time.Second) - continue - } - } - -} -*/ - func waitForPorterPS(ctx context.Context, ps *porterv1.ParameterSet, msg string) error { Log("%s: %s/%s", msg, ps.Namespace, ps.Name) key := client.ObjectKey{Namespace: ps.Namespace, Name: ps.Name} From 5a4261330527d19eec4b4d6158d62c56162e75c5 Mon Sep 17 00:00:00 2001 From: Steven Gettys Date: Wed, 25 May 2022 11:59:34 -0700 Subject: [PATCH 5/7] chore: Updated docs and added some test coverage Signed-off-by: Steven Gettys --- Makefile | 3 -- api/v1/parameterset_types_test.go | 21 ++++++---- api/v1/testdata/parameter-set.yaml | 3 ++ config/samples/_v1_parameterset.yaml | 11 ++++- config/samples/kustomization.yaml | 1 + controllers/parameterset_controller.go | 2 +- controllers/parameterset_controller_test.go | 6 +-- docs/content/file-formats.md | 45 +++++++++++++++++++++ docs/content/glossary.md | 22 ++++++++++ installer/vanilla.porter.yaml | 2 +- 10 files changed, 99 insertions(+), 17 deletions(-) delete mode 100644 Makefile diff --git a/Makefile b/Makefile deleted file mode 100644 index 85201921..00000000 --- a/Makefile +++ /dev/null @@ -1,3 +0,0 @@ -# target for controller-gen -generate: - mage -v GenerateController \ No newline at end of file diff --git a/api/v1/parameterset_types_test.go b/api/v1/parameterset_types_test.go index a6625bfc..2cbf29a7 100644 --- a/api/v1/parameterset_types_test.go +++ b/api/v1/parameterset_types_test.go @@ -33,10 +33,15 @@ func TestParameterSetSpec_ToPorterDocument(t *testing.T) { fields: fields{SchemaVersion: "1.0.1", Name: "porter-test-me", Namespace: "dev", - Parameters: []Parameter{{ - Name: "param1", - Source: ParameterSource{Value: "test-param"}, - }, + Parameters: []Parameter{ + { + Name: "param1", + Source: ParameterSource{Value: "test-param"}, + }, + { + Name: "param2", + Source: ParameterSource{Secret: "test-secret"}, + }, }, }, wantFile: wantGoldenFile, @@ -56,7 +61,7 @@ func TestParameterSetSpec_ToPorterDocument(t *testing.T) { got, err := cs.ToPorterDocument() if tt.wantErrMsg == "" { require.NoError(t, err) - portertest.CompareGoldenFile(t, "testdata/parameter-set.yaml", string(got)) + portertest.CompareGoldenFile(t, tt.wantFile, string(got)) } else { portertests.RequireErrorContains(t, err, tt.wantErrMsg) } @@ -92,14 +97,14 @@ func TestParameterSet_SetRetryAnnotation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cs := &ParameterSet{ + ps := &ParameterSet{ TypeMeta: tt.fields.TypeMeta, ObjectMeta: tt.fields.ObjectMeta, Spec: tt.fields.Spec, Status: tt.fields.Status, } - cs.SetRetryAnnotation(tt.args.retry) - assert.Equal(t, tt.args.retry, cs.Annotations[AnnotationRetry]) + ps.SetRetryAnnotation(tt.args.retry) + assert.Equal(t, tt.args.retry, ps.Annotations[AnnotationRetry]) }) } } diff --git a/api/v1/testdata/parameter-set.yaml b/api/v1/testdata/parameter-set.yaml index 7b15024a..424e869b 100644 --- a/api/v1/testdata/parameter-set.yaml +++ b/api/v1/testdata/parameter-set.yaml @@ -5,3 +5,6 @@ parameters: - name: param1 source: value: test-param + - name: param2 + source: + secret: test-secret diff --git a/config/samples/_v1_parameterset.yaml b/config/samples/_v1_parameterset.yaml index bbeff556..91945336 100644 --- a/config/samples/_v1_parameterset.yaml +++ b/config/samples/_v1_parameterset.yaml @@ -3,4 +3,13 @@ kind: ParameterSet metadata: name: parameterset-sample spec: - # TODO(user): Add fields here + schemaVersion: 1.0.1 + namespace: operator + name: porter-test-me + parameters: + - name: test-secret + source: + value: test-value + - name: test-secret + source: + secret: test-secret \ No newline at end of file diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index ddb119f6..7fcd6ef2 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -5,4 +5,5 @@ resources: - _v1_agentconfig.yaml - _v1_agentaction.yaml - _v1_credentialset.yaml +- _v1_parameterset.yaml # +kubebuilder:scaffold:manifestskustomizesamples diff --git a/controllers/parameterset_controller.go b/controllers/parameterset_controller.go index 7cef801e..d92bda14 100644 --- a/controllers/parameterset_controller.go +++ b/controllers/parameterset_controller.go @@ -44,6 +44,7 @@ func (r *ParameterSetReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +// Reconcile is called when the spec of a parameter set is changed func (r *ParameterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Log.WithValues("parameterSet", req.Name, "namespace", req.Namespace) @@ -163,7 +164,6 @@ func (r *ParameterSetReconciler) runParameterSet(ctx context.Context, log logr.L return r.runPorter(ctx, log, ps) } -// This could be the main "runFunction for each controller" // Trigger an agent func (r *ParameterSetReconciler) runPorter(ctx context.Context, log logr.Logger, ps *porterv1.ParameterSet) error { action, err := r.createAgentAction(ctx, log, ps) diff --git a/controllers/parameterset_controller_test.go b/controllers/parameterset_controller_test.go index 90a6e858..92918abf 100644 --- a/controllers/parameterset_controller_test.go +++ b/controllers/parameterset_controller_test.go @@ -63,7 +63,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { require.NotNil(t, ps.Status.Action, "expected Action to be set") var action porterv1.AgentAction require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: ps.Namespace, Name: ps.Status.Action.Name}, &action)) - assert.Equal(t, "1", action.Labels[porterv1.LabelResourceGeneration], "The wrong action is set on the status") + assert.Equal(t, "1", action.Labels[porterv1.LabelResourceGeneration], "The wrong resource generation is set for the agent action") // Mark the action as scheduled action.Status.Phase = porterv1.PhasePending @@ -97,7 +97,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { // Verify the parameter set status was synced with the action assert.NotNil(t, ps.Status.Action, "expected Action to still be set") assert.Equal(t, porterv1.PhaseSucceeded, ps.Status.Phase, "incorrect Phase") - assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(string(porterv1.ConditionComplete)))) + assert.True(t, apimeta.IsStatusConditionTrue(ps.Status.Conditions, string(porterv1.ConditionComplete))) // Fail the action action.Status.Phase = porterv1.PhaseFailed @@ -153,7 +153,7 @@ func TestParameterSetReconiler_Reconcile(t *testing.T) { // Verify that an action was created to delete it require.NotNil(t, ps.Status.Action, "expected Action to be set") require.NoError(t, controller.Get(ctx, client.ObjectKey{Namespace: ps.Namespace, Name: ps.Status.Action.Name}, &action)) - assert.Equal(t, "3", action.Labels[porterv1.LabelResourceGeneration], "The wrong action is set on the status") + assert.Equal(t, "3", action.Labels[porterv1.LabelResourceGeneration], "The wrong resource generation is set for the agent action") // Complete the delete action action.Status.Phase = porterv1.PhaseSucceeded diff --git a/docs/content/file-formats.md b/docs/content/file-formats.md index c241d32b..88701796 100644 --- a/docs/content/file-formats.md +++ b/docs/content/file-formats.md @@ -11,6 +11,7 @@ The same goes for the name and labels fields. * [Installation](#installation) * [CredentialSet](#credentialset) +* [ParameterSet](#parameterset) * [AgentAction](#agentaction) * [AgentConfig](#agentconfig) * [PorterConfig](#porterconfig) @@ -35,6 +36,11 @@ In addition to the normal fields available on a [Porter Installation document](/ See the glossary for more information about the [CredentialSet] resource. +The CredentialSet spec is the same schema as the CredentialSet resource in Porter. +You can copy/paste the output of the `porter credentials show NAME -o yaml` command into the CredentialSet resource spec (removing the status section). + +In addition to the normal fields available on a [Porter Credential Set document](/reference/file-formats/), the following fields are supported: + ```yaml apiVersion: porter.sh/v1 kind: CredentialSet @@ -61,6 +67,45 @@ spec: [CredentialSet]: /operator/glossary/#credentialset +## ParameterSet + +See the glossary for more information about the [ParameterSet] resource. + +The ParameterSet spec is the same schema as the ParameterSet resource in Porter. +You can copy/paste the output of the `porter parameters show NAME -o yaml` command into the ParameterSet resource spec (removing the status section). + +In addition to the normal fields available on a [Porter Parameter Set document](/reference/file-formats/), the following fields are supported: + + +```yaml +apiVersion: porter.sh/v1 +kind: ParameterSet +metadata: + name: parameterset-sample +spec: + schemaVersion: 1.0.1 + namespace: operator + name: porter-test-me + parameters: + - name: test-secret + source: + value: test-value + - name: test-secret + source: + secret: test-secret +``` + +| Field | Required | Default | Description | +|---------------------------|----------|------------------------------------|-------------------------------------------------------------| +| agentConfig | false | See [Agent Config](#agentconfig) | Reference to an AgentConfig resource in the same namespace. | +| porterConfig | false | See [Porter Config](#porterconfig) | Reference to a PorterConfig resource in the same namespace. | +| parameters | true | | List of parameter sources for the set | +| parameters.name | true | | The name of the parameter for the bundle | +| parameters.source | true | | The parameters type. Currently `vaule` and `secret` are the only supported sources | +| **oneof** `parameters.source.secret` `parameters.source.value` | true | | The plaintext value to use or the name of the secret that holds the parameter | + +[ParameterSet]: /operator/glossary/#parameterset + ## AgentAction See the glossary for more information about the [AgentAction] resource. diff --git a/docs/content/glossary.md b/docs/content/glossary.md index 1868cbed..233d2cba 100644 --- a/docs/content/glossary.md +++ b/docs/content/glossary.md @@ -59,6 +59,28 @@ or [Hashicorp Vault](/plugins/hashicorp/) are configured instead. The operator creates a corresponding AgentAction to create, update or delete Porter credentials. Once created the credential set is available to an Installation resource via its spec file. +### ParameterSet + +The [ParameterSet] custom resource represents a parameter set in Porter. + +[ParameterSet]: /operator/file-formats/#parameterset + +A ParameterSet supports a parameter source of `secret` for Porter secrets +plugins and `value` for plaintext values. + +Secrets source keys may vary depending on which [secret plugin](/plugins/) you have configured. +The [host secrets plugin](/plugins/host/) is not a good fit for use with the Porter Operator because environment variables or +files are not a recommended way to manage secrets on a cluster. +The [kubernetes.secrets plugin](https://release-v1.porter.sh/plugins/kubernetes/#secrets) +can retrieve secrets from native Kubernetes secrets, and otherwise we +recommend that an external secret store such as [Azure KeyVault](/plugins/azure/#secrets) +or [Hashicorp Vault](/plugins/hashicorp/) are configured instead. + +Value sources are stored in plaintext in the resource. + +The operator creates a corresponding AgentAction to create, update or delete Porter parameters. +Once created the parameter set is available to an Installation resource via its spec file. + ### AgentAction The [AgentAction] custom resource represents a Porter command that is run in the [PorterAgent](#porteragent). diff --git a/installer/vanilla.porter.yaml b/installer/vanilla.porter.yaml index 657701f1..5d463d7c 100644 --- a/installer/vanilla.porter.yaml +++ b/installer/vanilla.porter.yaml @@ -93,7 +93,7 @@ install: namespace: porter-operator-system name: mongodb chart: bitnami/mongodb - version: 10.27.2 + version: 12.1.15 set: auth.enabled: false - exec: From f9d4d7b1aa2edf0473026109be625881db644b0a Mon Sep 17 00:00:00 2001 From: Steven Gettys Date: Thu, 2 Jun 2022 16:20:12 -0700 Subject: [PATCH 6/7] chore: Added parameter for helm chart version configuration Signed-off-by: Steven Gettys --- installer/vanilla.porter.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/installer/vanilla.porter.yaml b/installer/vanilla.porter.yaml index 5d463d7c..e55ca541 100644 --- a/installer/vanilla.porter.yaml +++ b/installer/vanilla.porter.yaml @@ -62,6 +62,12 @@ parameters: default: "" applyTo: - configureNamespace + - name: mongodbChartVersion + description: Version of the mongodb helm chart to install + type: string + default: "12.1.15" + applyTo: + - install credentials: - name: kubeconfig @@ -93,7 +99,7 @@ install: namespace: porter-operator-system name: mongodb chart: bitnami/mongodb - version: 12.1.15 + version: "{{bundle.parameters.mongodbChartVersion}}" set: auth.enabled: false - exec: From 77121052b5158ff12b079e6da30a846f02b46241 Mon Sep 17 00:00:00 2001 From: Steven Gettys Date: Fri, 3 Jun 2022 08:55:17 -0700 Subject: [PATCH 7/7] docs: Fixed docs references Signed-off-by: Steven Gettys --- docs/content/file-formats.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/content/file-formats.md b/docs/content/file-formats.md index 88701796..659f6a52 100644 --- a/docs/content/file-formats.md +++ b/docs/content/file-formats.md @@ -23,7 +23,7 @@ See the glossary for more information about the [Installation] resource. The Installation spec is the same schema as the Installation resource in Porter. You can copy/paste the output of the `porter installation show NAME -o yaml` command into the Installation resource spec (removing the status section). -In addition to the normal fields available on a [Porter Installation document](/reference/file-formats/), the following fields are supported: +In addition to the normal fields available on a [Porter Installation document](/reference/file-formats/#installation), the following fields are supported: | Field | Required | Default | Description | |--------------|----------|-------------------------------------|-------------------------------------------------------------| @@ -39,7 +39,7 @@ See the glossary for more information about the [CredentialSet] resource. The CredentialSet spec is the same schema as the CredentialSet resource in Porter. You can copy/paste the output of the `porter credentials show NAME -o yaml` command into the CredentialSet resource spec (removing the status section). -In addition to the normal fields available on a [Porter Credential Set document](/reference/file-formats/), the following fields are supported: +In addition to the normal fields available on a [Porter Credential Set document](/reference/file-formats/#credential-set), the following fields are supported: ```yaml apiVersion: porter.sh/v1 @@ -74,7 +74,7 @@ See the glossary for more information about the [ParameterSet] resource. The ParameterSet spec is the same schema as the ParameterSet resource in Porter. You can copy/paste the output of the `porter parameters show NAME -o yaml` command into the ParameterSet resource spec (removing the status section). -In addition to the normal fields available on a [Porter Parameter Set document](/reference/file-formats/), the following fields are supported: +In addition to the normal fields available on a [Porter Parameter Set document](/reference/file-formats/#parameter-set), the following fields are supported: ```yaml