diff --git a/api/v1alpha1/resource_group.go b/api/v1alpha1/resource_group.go index 9464b9fa..c1858472 100644 --- a/api/v1alpha1/resource_group.go +++ b/api/v1alpha1/resource_group.go @@ -72,6 +72,8 @@ type Resource struct { Name string `json:"name,omitempty"` // +kubebuilder:validation:Required Definition runtime.RawExtension `json:"definition,omitempty"` + // +kubebuilder:validation:Optional + ReadyOn []string `json:"readyOn,omitempty"` } // ResourceGroupStatus defines the observed state of ResourceGroup diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 18d0cbe9..d0762012 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -77,6 +77,11 @@ func (in *Definition) DeepCopy() *Definition { func (in *Resource) DeepCopyInto(out *Resource) { *out = *in in.Definition.DeepCopyInto(&out.Definition) + if in.ReadyOn != nil { + in, out := &in.ReadyOn, &out.ReadyOn + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Resource. diff --git a/config/crd/bases/x.symphony.k8s.aws_resourcegroups.yaml b/config/crd/bases/x.symphony.k8s.aws_resourcegroups.yaml index 2fe5b5e2..696fe0f9 100644 --- a/config/crd/bases/x.symphony.k8s.aws_resourcegroups.yaml +++ b/config/crd/bases/x.symphony.k8s.aws_resourcegroups.yaml @@ -110,6 +110,11 @@ spec: x-kubernetes-preserve-unknown-fields: true name: type: string + readyOn: + items: + type: string + x-kubernetes-preserve-unknown-fields: true + type: array required: - definition - name diff --git a/examples/ack-controller/ec2-controller/ec2-controller-instance.yaml b/examples/ack-controller/ec2-controller/ec2-controller-instance.yaml index f1b3bedc..a6649d77 100644 --- a/examples/ack-controller/ec2-controller/ec2-controller-instance.yaml +++ b/examples/ack-controller/ec2-controller/ec2-controller-instance.yaml @@ -1,5 +1,5 @@ apiVersion: x.symphony.k8s.aws/v1alpha1 -kind: EC2controller +kind: EC2Controller metadata: name: my-symphony-ec2-controller namespace: default diff --git a/examples/ack-controller/eks-controller/eks-controller-instance.yaml b/examples/ack-controller/eks-controller/eks-controller-instance.yaml index 728761b1..b082d795 100644 --- a/examples/ack-controller/eks-controller/eks-controller-instance.yaml +++ b/examples/ack-controller/eks-controller/eks-controller-instance.yaml @@ -1,5 +1,5 @@ apiVersion: x.symphony.k8s.aws/v1alpha1 -kind: EKScontroller +kind: EKSController metadata: name: my-symphony-eks-controller namespace: default @@ -12,7 +12,7 @@ spec: deployment: {} iamRole: oidcProvider: oidc.eks.us-west-2.amazonaws.com/id/50B8942190FBD3A2EF2BF6AB7D27B06B - iamRolePolicy: {} + iamPolicy: {} image: resources: requests: {} diff --git a/examples/ack-eks-cluster/eks-cluster.yaml b/examples/ack-eks-cluster/eks-cluster.yaml index 7325f53d..18549583 100644 --- a/examples/ack-eks-cluster/eks-cluster.yaml +++ b/examples/ack-eks-cluster/eks-cluster.yaml @@ -19,6 +19,8 @@ spec: # resources resources: - name: clusterVPC + readyOn: + - ${status.state == "available"} definition: apiVersion: ec2.services.k8s.aws/v1alpha1 kind: VPC @@ -56,6 +58,8 @@ spec: - destinationCIDRBlock: 0.0.0.0/0 gatewayID: ${clusterInternetGateway.status.internetGatewayID} - name: clusterSubnetA + readyOn: + - ${status.state == "available"} definition: apiVersion: ec2.services.k8s.aws/v1alpha1 kind: Subnet @@ -169,6 +173,8 @@ spec: ] } - name: cluster + readyOn: + - ${status.status == "ACTIVE"} definition: apiVersion: eks.services.k8s.aws/v1alpha1 kind: Cluster diff --git a/examples/deployment-service/deployment-service.yaml b/examples/deployment-service/deployment-service.yaml index fd6620c9..5db21e0b 100644 --- a/examples/deployment-service/deployment-service.yaml +++ b/examples/deployment-service/deployment-service.yaml @@ -14,13 +14,15 @@ spec: availableReplicas: ${deployment.status.availableReplicas} resources: - name: deployment + readyOn: + - ${spec.replicas == status.availableReplicas} definition: apiVersion: apps/v1 kind: Deployment metadata: name: ${spec.name} spec: - replicas: 1 + replicas: 50 selector: matchLabels: app: deployment diff --git a/internal/celutil/conversions.go b/internal/celutil/conversions.go index 93f5a9b4..0aaacd2f 100644 --- a/internal/celutil/conversions.go +++ b/internal/celutil/conversions.go @@ -48,3 +48,7 @@ func ConvertCELtoGo(v ref.Val) (interface{}, error) { return v.Value(), fmt.Errorf("unsupported type: %v", v.Type()) } } + +func IsBoolType(v ref.Val) bool { + return v.Type() == types.BoolType +} diff --git a/internal/controller/instance/controller_reconcile.go b/internal/controller/instance/controller_reconcile.go index 71958c2b..6dd610a0 100644 --- a/internal/controller/instance/controller_reconcile.go +++ b/internal/controller/instance/controller_reconcile.go @@ -213,6 +213,20 @@ func (igr *InstanceGraphReconciler) reconcileResource(ctx context.Context, resou } igr.runtime.SetLatestResource(resourceID, observed) + + log.V(1).Info("Checking if resource is Ready", "resource", resourceID) + if ready, err := igr.runtime.IsResourceReady(resourceID); err != nil { + log.V(1).Info("Resource not ready", "resource", resourceID) + resourceState.State = "WaitingForReadiness" + resourceState.Err = fmt.Errorf("resource not ready: %w", err) + return igr.delayedRequeue(resourceState.Err) + } else if !ready { + log.V(1).Info("Resource not ready", "resource", resourceID) + resourceState.State = "WaitingForReadiness" + resourceState.Err = fmt.Errorf("resource not ready") + return igr.delayedRequeue(resourceState.Err) + } + return igr.updateResource(ctx, rc, rUnstructured, observed, resourceID, resourceState) } diff --git a/internal/resourcegroup/builder.go b/internal/resourcegroup/builder.go index 581b552e..0227475e 100644 --- a/internal/resourcegroup/builder.go +++ b/internal/resourcegroup/builder.go @@ -174,17 +174,28 @@ func (b *GraphBuilder) NewResourceGroup(rg *v1alpha1.ResourceGroup) (*ResourceGr }) } } + // 6. Parse ReadyOn expressions + readyOn, err := parser.ParseConditionExpressions(rgResource.ReadyOn) + if err != nil { + return nil, fmt.Errorf("failed to parse readyOn expressions: %v", err) + } + + readyOnExpressions := []ReadyOnExpression{} + for _, e := range readyOn { + readyOnExpressions = append(readyOnExpressions, ReadyOnExpression{Resolved: false, Expression: e}) + } _, isNamespaced := namespacedResources[gvk] resources[rgResource.Name] = &Resource{ - ID: rgResource.Name, - GroupVersionKind: gvk, - Schema: resourceSchema, - EmulatedObject: emulatedResource, - OriginalObject: unstructuredResource, - Variables: resourceVariables, - Namespaced: isNamespaced, + ID: rgResource.Name, + GroupVersionKind: gvk, + Schema: resourceSchema, + EmulatedObject: emulatedResource, + OriginalObject: unstructuredResource, + Variables: resourceVariables, + ReadyOnExpressions: readyOnExpressions, + Namespaced: isNamespaced, } } @@ -335,6 +346,48 @@ func (b *GraphBuilder) validateResourceCELExpressions(resources map[string]*Reso return fmt.Errorf("failed to dry-run expression %s: %w", expression, err) } } + // validate readyOn Expressions for resource + // Only accepting expressions accessing the status and spec for now + // and need to evaluate to a boolean type + // + // TODO(michaelhtm) It shares some of the logic with the loop from above..maybe + // we can refactor them or put it in one function. + // I would also suggest separating the dryRuns of readyOnExpressions + // and the resourceExpressions. + for _, expression := range resource.ReadyOnExpressions { + fieldNames := getResourceTopLevelFieldNames(resource.Schema) + fieldEnv, err := celutil.NewEnvironement(celutil.WithResourceNames(fieldNames)) + if err != nil { + return fmt.Errorf("failed to create CEL environment: %w", err) + } + + err = b.validateCELExpressionContext(fieldEnv, expression.Expression, fieldNames) + if err != nil { + return fmt.Errorf("failed to validate expression context: '%s' %w", expression.Expression, err) + } + // create context + // add resource fields to the context + context := map[string]*Resource{} + for _, n := range fieldNames { + context[n] = &Resource{ + ID: n, + GroupVersionKind: resource.GroupVersionKind, + Schema: resource.Schema, + EmulatedObject: &unstructured.Unstructured{ + Object: resource.EmulatedObject.Object[n].(map[string]interface{}), + }, + } + } + + output, err := b.dryRunExpression(fieldEnv, expression.Expression, context) + + if err != nil { + return fmt.Errorf("failed to dry-run expression %s: %w", expression.Expression, err) + } + if !celutil.IsBoolType(output) { + return fmt.Errorf("output of readyOn expression %s can only be of type bool", expression.Expression) + } + } } } @@ -501,7 +554,7 @@ func (b *GraphBuilder) buildInstanceResource( instanceVariables := []*ResourceVariable{} for _, statusVariable := range statusVariables { // These variables needs to be injected into the status field of the instance. - path := ".status" + statusVariable.Path + path := ">status" + statusVariable.Path statusVariable.Path = path instanceVariables = append(instanceVariables, &ResourceVariable{ Kind: ResourceVariableKindDynamic, diff --git a/internal/resourcegroup/ready_on_expression.go b/internal/resourcegroup/ready_on_expression.go new file mode 100644 index 00000000..42cf3e7f --- /dev/null +++ b/internal/resourcegroup/ready_on_expression.go @@ -0,0 +1,48 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resourcegroup + +import ( + "slices" + + "k8s.io/kube-openapi/pkg/validation/spec" +) + +type ReadyOnExpression struct { + Resolved bool + Expression string +} + +// This function goes through the schema of a resource +// and retrieves the field names fo rthe readOnly feature +// +// Currently we support all the top level fields besides +// apiVersion and kind. If we do see a case where they would +// be necessary for readyOns, we can include them here. +func getResourceTopLevelFieldNames(schema *spec.Schema) []string { + + fieldNames := []string{} + + if schema == nil || schema.Properties == nil { + return fieldNames + } + for k, _ := range schema.Properties { + if k != "apiVersion" && k != "kind" { + fieldNames = append(fieldNames, k) + } + } + + slices.Sort(fieldNames) + return fieldNames +} diff --git a/internal/resourcegroup/resource.go b/internal/resourcegroup/resource.go index edbf95d0..cd5f877f 100644 --- a/internal/resourcegroup/resource.go +++ b/internal/resourcegroup/resource.go @@ -35,6 +35,8 @@ type Resource struct { Variables []*ResourceVariable + ReadyOnExpressions []ReadyOnExpression + Dependencies []string Namespaced bool } diff --git a/internal/resourcegroup/runtime.go b/internal/resourcegroup/runtime.go index fd4622da..6f10a272 100644 --- a/internal/resourcegroup/runtime.go +++ b/internal/resourcegroup/runtime.go @@ -104,9 +104,64 @@ func (rt *RuntimeResourceGroup) CanResolveResource(resource string) bool { return false } } + return true } +func (rt *RuntimeResourceGroup) IsResourceReady(resourceID string) (bool, error) { + + expressions := rt.ResourceGroup.Resources[resourceID].ReadyOnExpressions + resource := rt.ResolvedResources[resourceID] + fieldNames := getResourceTopLevelFieldNames(rt.ResourceGroup.Resources[resourceID].Schema) + if len(expressions) == 0 { + return true, nil + } + + // fieldNames := []string{"status", "spec"} + env, err := celutil.NewEnvironement(&celutil.EnvironementOptions{ + ResourceNames: fieldNames, + }) + // we should not expect errors here since we already compiled it + // in the dryRun + if err != nil { + return false, fmt.Errorf("failed creating new Environment: %w", err) + } + context := map[string]interface{}{} + for _, n := range fieldNames { + if obj, ok := resource.Object[n]; ok { + context[n] = obj.(map[string]interface{}) + } + } + for _, e := range expressions { + if e.Resolved { + continue + } + ast, issues := env.Compile(e.Expression) + if issues != nil && issues.Err() != nil { + return false, fmt.Errorf("failed compiling expression %s: %w", e.Expression, err) + } + program, err := env.Program(ast) + if err != nil { + return false, fmt.Errorf("failed programming expression %s: %w", e.Expression, err) + } + + output, _, err := program.Eval(context) + if err != nil { + return false, fmt.Errorf("failed evaluating expression %s: %w", e.Expression, err) + } + out, err := celutil.ConvertCELtoGo(output) + if err != nil { + return false, fmt.Errorf("failed converting output %v: %w", output, err) + } + // keep checking for a false + if !out.(bool) { + return false, nil + } + e.Resolved = out.(bool) + } + return true, err +} + func (rt *RuntimeResourceGroup) SetLatestResource(name string, resource *unstructured.Unstructured) { rt.ResolvedResources[name] = resource } diff --git a/internal/typesystem/parser/conditions.go b/internal/typesystem/parser/conditions.go new file mode 100644 index 00000000..dc6e18fb --- /dev/null +++ b/internal/typesystem/parser/conditions.go @@ -0,0 +1,45 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package parser + +import ( + "fmt" + "strings" +) + +// This function parses resource condition expressions. +// These expressions need to be standalone expressions +// so, this function also does some validation. +// At the end we return the expressions with '${}' removed +// +// To be honest I wouldn't necessarily call it parse, since +// we are mostly just validating, without caring what's in +// the expression. Maybe we can rename it in the future 🤔 +func ParseConditionExpressions(conditions []string) ([]string, error) { + + expressions := make([]string, 0, len(conditions)) + + for _, e := range conditions { + ok, err := isOneShotExpression(e) + if err != nil { + return nil, err + } + if !ok { + return nil, fmt.Errorf("single expression per line allowed") + } + expressions = append(expressions, strings.Trim(e, "${}")) + } + + return expressions, nil +} diff --git a/internal/typesystem/parser/conditions_test.go b/internal/typesystem/parser/conditions_test.go new file mode 100644 index 00000000..da7a023e --- /dev/null +++ b/internal/typesystem/parser/conditions_test.go @@ -0,0 +1,70 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package parser + +import ( + "testing" +) + +func TestParseReadyOn(t *testing.T) { + testCases := []struct { + name string + expression []string + expectedError string + }{ + { + name: "Two expressions", + expression: []string{"${hello}${goodbye}"}, + expectedError: "single expression per line allowed", + }, + { + name: "With Postfix", + expression: []string{"${hello}-world"}, + expectedError: "single expression per line allowed", + }, + { + name: "With Prefix", + expression: []string{"hello-${world}"}, + expectedError: "single expression per line allowed", + }, + { + name: "Standalone expression", + expression: []string{"${hello}"}, + expectedError: "", + }, + { + name: "Complex standalone expression that works", + expression: []string{"${hello + world - someone = whoever[else]isNone dum dum {pop}out-here}"}, + expectedError: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := ParseConditionExpressions(tc.expression) + + if tc.expectedError == "" { + if err != nil { + t.Error("Expected no error, but got '%w'", err) + } + } else { + if err == nil { + t.Errorf("Expected error '%s', but got nil", tc.expectedError) + } else if err.Error() != tc.expectedError { + t.Errorf("Expected error: %s\nError we got: %s", tc.expectedError, err.Error()) + } + } + }) + } +}