From 76d630a64a72ba97df5ccf58ce98afe84e479c52 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Fri, 4 Oct 2024 12:05:27 +0100 Subject: [PATCH] User Tasks: add `discover-ec2` task type (#47062) * User Tasks: add `discover-ec2` task type This PR adds more business logic into the User Tasks, in particular into the `discover-ec2` task type. One of the key features of the DiscoverEC2 User Tasks is that we must have a single task per: - integration - region - account id - issue type This allows user to have a detailed view of the issues their are facing but still grouping EC2 instances. To do this, we had to move the region and account id up one level. Previously they were at the instance level, and it would require iterating over them to actually create the group (uniq key) we want. This also adds well known errors as issue types to ensure we validate them. A later PR will come where we actually start creating/updating DiscoverEC2 User Tasks from the DiscoveryService. * use strings.Compare * improve docs and fix typos --- .../go/teleport/usertasks/v1/user_tasks.pb.go | 74 +++--- .../teleport/usertasks/v1/user_tasks.proto | 12 +- api/types/usertasks/object.go | 206 ++++++++++++++-- api/types/usertasks/object_test.go | 232 ++++++++++++++++-- lib/cache/cache_test.go | 39 +-- lib/services/local/user_task_test.go | 74 +++--- lib/services/user_task_test.go | 45 ++-- 7 files changed, 536 insertions(+), 146 deletions(-) diff --git a/api/gen/proto/go/teleport/usertasks/v1/user_tasks.pb.go b/api/gen/proto/go/teleport/usertasks/v1/user_tasks.pb.go index d5801315904ca..73785724fa686 100644 --- a/api/gen/proto/go/teleport/usertasks/v1/user_tasks.pb.go +++ b/api/gen/proto/go/teleport/usertasks/v1/user_tasks.pb.go @@ -224,6 +224,10 @@ type DiscoverEC2 struct { // Instances maps an instance id to the result of enrolling that instance into teleport. Instances map[string]*DiscoverEC2Instance `protobuf:"bytes,1,rep,name=instances,proto3" json:"instances,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` + // AccountID is the AWS Account ID for the instances. + AccountId string `protobuf:"bytes,2,opt,name=account_id,json=accountId,proto3" json:"account_id,omitempty"` + // Region is the AWS Region where Teleport failed to enroll EC2 instances. + Region string `protobuf:"bytes,3,opt,name=region,proto3" json:"region,omitempty"` } func (x *DiscoverEC2) Reset() { @@ -265,6 +269,20 @@ func (x *DiscoverEC2) GetInstances() map[string]*DiscoverEC2Instance { return nil } +func (x *DiscoverEC2) GetAccountId() string { + if x != nil { + return x.AccountId + } + return "" +} + +func (x *DiscoverEC2) GetRegion() string { + if x != nil { + return x.Region + } + return "" +} + // DiscoverEC2Instance contains the result of enrolling an AWS EC2 Instance. type DiscoverEC2Instance struct { state protoimpl.MessageState @@ -276,10 +294,6 @@ type DiscoverEC2Instance struct { // Name is the instance Name. // Might be empty, if the instance doesn't have the Name tag. Name string `protobuf:"bytes,2,opt,name=name,proto3" json:"name,omitempty"` - // AccountID is the AWS Account ID for this instance. - AccountId string `protobuf:"bytes,3,opt,name=account_id,json=accountId,proto3" json:"account_id,omitempty"` - // Region is the AWS Region where this issue is happening. - Region string `protobuf:"bytes,4,opt,name=region,proto3" json:"region,omitempty"` // InvocationURL is the URL that points to the invocation. // Empty if there was an error before installing the InvocationUrl string `protobuf:"bytes,5,opt,name=invocation_url,json=invocationUrl,proto3" json:"invocation_url,omitempty"` @@ -337,20 +351,6 @@ func (x *DiscoverEC2Instance) GetName() string { return "" } -func (x *DiscoverEC2Instance) GetAccountId() string { - if x != nil { - return x.AccountId - } - return "" -} - -func (x *DiscoverEC2Instance) GetRegion() string { - if x != nil { - return x.Region - } - return "" -} - func (x *DiscoverEC2Instance) GetInvocationUrl() string { if x != nil { return x.InvocationUrl @@ -415,28 +415,28 @@ var file_teleport_usertasks_v1_user_tasks_proto_rawDesc = []byte{ 0x32, 0x18, 0x05, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x22, 0x2e, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2e, 0x75, 0x73, 0x65, 0x72, 0x74, 0x61, 0x73, 0x6b, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x44, 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x43, 0x32, 0x52, 0x0b, 0x64, 0x69, 0x73, - 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x63, 0x32, 0x22, 0xc8, 0x01, 0x0a, 0x0b, 0x44, 0x69, 0x73, + 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x63, 0x32, 0x22, 0xff, 0x01, 0x0a, 0x0b, 0x44, 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x43, 0x32, 0x12, 0x4f, 0x0a, 0x09, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x31, 0x2e, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2e, 0x75, 0x73, 0x65, 0x72, 0x74, 0x61, 0x73, 0x6b, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x44, 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x43, 0x32, 0x2e, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x73, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x52, 0x09, - 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x73, 0x1a, 0x68, 0x0a, 0x0e, 0x49, 0x6e, 0x73, - 0x74, 0x61, 0x6e, 0x63, 0x65, 0x73, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, - 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x40, 0x0a, - 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x2a, 0x2e, 0x74, - 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2e, 0x75, 0x73, 0x65, 0x72, 0x74, 0x61, 0x73, 0x6b, - 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x44, 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x43, 0x32, - 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, - 0x02, 0x38, 0x01, 0x22, 0xb5, 0x02, 0x0a, 0x13, 0x44, 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, - 0x45, 0x43, 0x32, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x12, 0x1f, 0x0a, 0x0b, 0x69, - 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, - 0x52, 0x0a, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x49, 0x64, 0x12, 0x12, 0x0a, 0x04, - 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, - 0x12, 0x1d, 0x0a, 0x0a, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x03, - 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x49, 0x64, 0x12, - 0x16, 0x0a, 0x06, 0x72, 0x65, 0x67, 0x69, 0x6f, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, - 0x06, 0x72, 0x65, 0x67, 0x69, 0x6f, 0x6e, 0x12, 0x25, 0x0a, 0x0e, 0x69, 0x6e, 0x76, 0x6f, 0x63, + 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x73, 0x12, 0x1d, 0x0a, 0x0a, 0x61, 0x63, 0x63, + 0x6f, 0x75, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x61, + 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x49, 0x64, 0x12, 0x16, 0x0a, 0x06, 0x72, 0x65, 0x67, 0x69, + 0x6f, 0x6e, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x72, 0x65, 0x67, 0x69, 0x6f, 0x6e, + 0x1a, 0x68, 0x0a, 0x0e, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x73, 0x45, 0x6e, 0x74, + 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x03, 0x6b, 0x65, 0x79, 0x12, 0x40, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, + 0x01, 0x28, 0x0b, 0x32, 0x2a, 0x2e, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2e, 0x75, + 0x73, 0x65, 0x72, 0x74, 0x61, 0x73, 0x6b, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x44, 0x69, 0x73, 0x63, + 0x6f, 0x76, 0x65, 0x72, 0x45, 0x43, 0x32, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x52, + 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0x9e, 0x02, 0x0a, 0x13, 0x44, + 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x45, 0x43, 0x32, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, + 0x63, 0x65, 0x12, 0x1f, 0x0a, 0x0b, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x5f, 0x69, + 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, + 0x65, 0x49, 0x64, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x25, 0x0a, 0x0e, 0x69, 0x6e, 0x76, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x5f, 0x75, 0x72, 0x6c, 0x18, 0x05, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, 0x69, 0x6e, 0x76, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x55, 0x72, 0x6c, 0x12, 0x29, 0x0a, 0x10, 0x64, 0x69, 0x73, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x79, 0x5f, 0x63, 0x6f, 0x6e, 0x66, @@ -447,7 +447,9 @@ var file_teleport_usertasks_v1_user_tasks_proto_rawDesc = []byte{ 0x75, 0x70, 0x12, 0x37, 0x0a, 0x09, 0x73, 0x79, 0x6e, 0x63, 0x5f, 0x74, 0x69, 0x6d, 0x65, 0x18, 0x08, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, - 0x70, 0x52, 0x08, 0x73, 0x79, 0x6e, 0x63, 0x54, 0x69, 0x6d, 0x65, 0x42, 0x56, 0x5a, 0x54, 0x67, + 0x70, 0x52, 0x08, 0x73, 0x79, 0x6e, 0x63, 0x54, 0x69, 0x6d, 0x65, 0x4a, 0x04, 0x08, 0x03, 0x10, + 0x04, 0x4a, 0x04, 0x08, 0x04, 0x10, 0x05, 0x52, 0x0a, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, + 0x5f, 0x69, 0x64, 0x52, 0x06, 0x72, 0x65, 0x67, 0x69, 0x6f, 0x6e, 0x42, 0x56, 0x5a, 0x54, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x72, 0x61, 0x76, 0x69, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, diff --git a/api/proto/teleport/usertasks/v1/user_tasks.proto b/api/proto/teleport/usertasks/v1/user_tasks.proto index f7c2e8612a483..81d8d52d17b63 100644 --- a/api/proto/teleport/usertasks/v1/user_tasks.proto +++ b/api/proto/teleport/usertasks/v1/user_tasks.proto @@ -63,19 +63,23 @@ message UserTaskSpec { message DiscoverEC2 { // Instances maps an instance id to the result of enrolling that instance into teleport. map instances = 1; + // AccountID is the AWS Account ID for the instances. + string account_id = 2; + // Region is the AWS Region where Teleport failed to enroll EC2 instances. + string region = 3; } // DiscoverEC2Instance contains the result of enrolling an AWS EC2 Instance. message DiscoverEC2Instance { + // AccountID and Region were moved into the DiscoverEC2 message. + reserved 3, 4; + reserved "account_id", "region"; + // InstanceID is the EC2 Instance ID that uniquely identifies the instance. string instance_id = 1; // Name is the instance Name. // Might be empty, if the instance doesn't have the Name tag. string name = 2; - // AccountID is the AWS Account ID for this instance. - string account_id = 3; - // Region is the AWS Region where this issue is happening. - string region = 4; // InvocationURL is the URL that points to the invocation. // Empty if there was an error before installing the string invocation_url = 5; diff --git a/api/types/usertasks/object.go b/api/types/usertasks/object.go index 978d10c3d1121..df00f4e0a58a1 100644 --- a/api/types/usertasks/object.go +++ b/api/types/usertasks/object.go @@ -19,32 +19,66 @@ package usertasks import ( + "encoding/binary" + "slices" + "time" + + "github.com/google/uuid" "github.com/gravitational/trace" + "google.golang.org/protobuf/types/known/timestamppb" headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1" usertasksv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/usertasks/v1" "github.com/gravitational/teleport/api/types" ) -// NewUserTask creates a new UserTask object. -// It validates the object before returning it. -func NewUserTask(name string, spec *usertasksv1.UserTaskSpec) (*usertasksv1.UserTask, error) { - cj := &usertasksv1.UserTask{ +// UserTaskOption defines a function that mutates a User Task. +type UserTaskOption func(ut *usertasksv1.UserTask) + +// WithExpiration sets the expiration of the UserTask resource. +func WithExpiration(t time.Time) func(ut *usertasksv1.UserTask) { + return func(ut *usertasksv1.UserTask) { + ut.Metadata.Expires = timestamppb.New(t) + } +} + +// NewDiscoverEC2UserTask creates a new DiscoverEC2 User Task Type. +func NewDiscoverEC2UserTask(spec *usertasksv1.UserTaskSpec, opts ...UserTaskOption) (*usertasksv1.UserTask, error) { + taskName := TaskNameForDiscoverEC2(TaskNameForDiscoverEC2Parts{ + Integration: spec.GetIntegration(), + IssueType: spec.GetIssueType(), + AccountID: spec.GetDiscoverEc2().GetAccountId(), + Region: spec.GetDiscoverEc2().GetRegion(), + }) + + ut := &usertasksv1.UserTask{ Kind: types.KindUserTask, Version: types.V1, Metadata: &headerv1.Metadata{ - Name: name, + Name: taskName, }, Spec: spec, } + for _, o := range opts { + o(ut) + } - if err := ValidateUserTask(cj); err != nil { + if err := ValidateUserTask(ut); err != nil { return nil, trace.Wrap(err) } - return cj, nil + return ut, nil } +const ( + // TaskStateOpen identifies an issue with an instance that is not yet resolved. + TaskStateOpen = "OPEN" + // TaskStateResolved identifies an issue with an instance that is resolved. + TaskStateResolved = "RESOLVED" +) + +var validTaskStates = []string{TaskStateOpen, TaskStateResolved} + const ( // TaskTypeDiscoverEC2 identifies a User Tasks that is created // when an auto-enrollment of an EC2 instance fails. @@ -52,44 +86,170 @@ const ( TaskTypeDiscoverEC2 = "discover-ec2" ) +// List of Auto Discover EC2 issues identifiers. +// This value is used to populate the UserTasks.Spec.IssueType for Discover EC2 tasks. +// The Web UI will then use those identifiers to show detailed instructions on how to fix the issue. +const ( + // AutoDiscoverEC2IssueEICEFailedToCreateNode is used when the EICE flow fails to create a node. + // This can happen when the Node does not have a valid PrivateIPAddress. + // This is very unlikely and should only happen if the AWS API returns an unexpected response. + AutoDiscoverEC2IssueEICEFailedToCreateNode = "ec2-eice-create-node" + + // AutoDiscoverEC2IssueEICEFailedToUpsertNode is used when the EICE flow fails to upsert a node into the cluster. + // This is very unlikely and should only happen + // - if the Discovery system role was changed + // - if the Node resource validation was changed on the Auth and not on the DiscoveryService + // - if Teleport backend is offline or in failing mode + // - or because of a network error + AutoDiscoverEC2IssueEICEFailedToUpsertNode = "ec2-eice-upsert-node" + + // AutoDiscoverEC2IssueScriptInstanceNotRegistered is used to identify instances that failed to auto-enroll + // because they are not present in Amazon Systems Manager. + // This usually means that the Instance does not have the SSM Agent running, + // or that the instance's IAM Profile does not allow have the managed IAM Policy AmazonSSMManagedInstanceCore assigned to it. + AutoDiscoverEC2IssueScriptInstanceNotRegistered = "ec2-ssm-agent-not-registered" + + // AutoDiscoverEC2IssueScriptInstanceConnectionLost is used to identify instances that failed to auto-enroll + // because the agent lost connection to Amazon Systems Manager. + // This can happen if the user changed some setting in the instance's network or IAM profile. + AutoDiscoverEC2IssueScriptInstanceConnectionLost = "ec2-ssm-agent-connection-lost" + + // AutoDiscoverEC2IssueScriptInstanceUnsupportedOS is used to identify instances that failed to auto-enroll + // because its OS is not supported by teleport. + // This can happen if the instance is running Windows. + AutoDiscoverEC2IssueScriptInstanceUnsupportedOS = "ec2-ssm-unsupported-os" + + // AutoDiscoverEC2IssueScriptFailure is used to identify instances that failed to auto-enroll + // because the installation script failed. + // The invocation url must be included in the report, so that users can see what was wrong. + AutoDiscoverEC2IssueScriptFailure = "ec2-ssm-script-failure" + + // AutoDiscoverEC2IssueInvocationFailure is used to identify instances that failed to auto-enroll + // because the SSM Script Run (also known as Invocation) failed. + // This happens when there's a failure with permissions or an invalid configuration (eg, invalid document name). + AutoDiscoverEC2IssueInvocationFailure = "ec2-ssm-invocation-failure" +) + +// discoverEC2IssueTypes is a list of issue types that can occur when trying to auto enroll EC2 instances. +var discoverEC2IssueTypes = []string{ + AutoDiscoverEC2IssueEICEFailedToCreateNode, + AutoDiscoverEC2IssueEICEFailedToUpsertNode, + AutoDiscoverEC2IssueScriptInstanceNotRegistered, + AutoDiscoverEC2IssueScriptInstanceConnectionLost, + AutoDiscoverEC2IssueScriptInstanceUnsupportedOS, + AutoDiscoverEC2IssueScriptFailure, + AutoDiscoverEC2IssueInvocationFailure, +} + // ValidateUserTask validates the UserTask object without modifying it. -func ValidateUserTask(uit *usertasksv1.UserTask) error { +func ValidateUserTask(ut *usertasksv1.UserTask) error { switch { - case uit.GetKind() != types.KindUserTask: + case ut.GetKind() != types.KindUserTask: return trace.BadParameter("invalid kind") - case uit.GetVersion() != types.V1: + case ut.GetVersion() != types.V1: return trace.BadParameter("invalid version") - case uit.GetSubKind() != "": + case ut.GetSubKind() != "": return trace.BadParameter("invalid sub kind, must be empty") - case uit.GetMetadata() == nil: + case ut.GetMetadata() == nil: return trace.BadParameter("user task metadata is nil") - case uit.Metadata.GetName() == "": + case ut.Metadata.GetName() == "": return trace.BadParameter("user task name is empty") - case uit.GetSpec() == nil: + case ut.GetSpec() == nil: return trace.BadParameter("user task spec is nil") - case uit.GetSpec().Integration == "": - return trace.BadParameter("integration is required") + case !slices.Contains(validTaskStates, ut.GetSpec().State): + return trace.BadParameter("invalid task state, allowed values: %v", validTaskStates) } - switch uit.Spec.TaskType { + switch ut.Spec.TaskType { case TaskTypeDiscoverEC2: - if err := validateDiscoverEC2TaskType(uit); err != nil { + if err := validateDiscoverEC2TaskType(ut); err != nil { return trace.Wrap(err) } default: - return trace.BadParameter("task type %q is not valid", uit.Spec.TaskType) + return trace.BadParameter("task type %q is not valid", ut.Spec.TaskType) } return nil } -func validateDiscoverEC2TaskType(uit *usertasksv1.UserTask) error { - if uit.Spec.DiscoverEc2 == nil { +func validateDiscoverEC2TaskType(ut *usertasksv1.UserTask) error { + if ut.GetSpec().Integration == "" { + return trace.BadParameter("integration is required") + } + if ut.GetSpec().DiscoverEc2 == nil { return trace.BadParameter("%s requires the discover_ec2 field", TaskTypeDiscoverEC2) } - if uit.Spec.IssueType == "" { - return trace.BadParameter("issue type is required") + if ut.GetSpec().DiscoverEc2.AccountId == "" { + return trace.BadParameter("%s requires the discover_ec2.account_id field", TaskTypeDiscoverEC2) + } + if ut.GetSpec().DiscoverEc2.Region == "" { + return trace.BadParameter("%s requires the discover_ec2.region field", TaskTypeDiscoverEC2) + } + + expectedTaskName := TaskNameForDiscoverEC2(TaskNameForDiscoverEC2Parts{ + Integration: ut.Spec.Integration, + IssueType: ut.Spec.IssueType, + AccountID: ut.Spec.DiscoverEc2.AccountId, + Region: ut.Spec.DiscoverEc2.Region, + }) + if ut.Metadata.GetName() != expectedTaskName { + return trace.BadParameter("task name is pre-defined for discover-ec2 types, expected %q, got %q", + expectedTaskName, + ut.Metadata.GetName(), + ) + } + + if !slices.Contains(discoverEC2IssueTypes, ut.GetSpec().IssueType) { + return trace.BadParameter("invalid issue type state, allowed values: %v", discoverEC2IssueTypes) + } + + if len(ut.Spec.DiscoverEc2.Instances) == 0 { + return trace.BadParameter("at least one instance is required") + } + for instanceID, instanceIssue := range ut.Spec.DiscoverEc2.Instances { + if instanceID == "" { + return trace.BadParameter("instance id in discover_ec2.instances map is required") + } + if instanceIssue.InstanceId == "" { + return trace.BadParameter("instance id in discover_ec2.instances field is required") + } + if instanceID != instanceIssue.InstanceId { + return trace.BadParameter("instance id in discover_ec2.instances map and field are different") + } + if instanceIssue.DiscoveryConfig == "" { + return trace.BadParameter("discovery config in discover_ec2.instances field is required") + } + if instanceIssue.DiscoveryGroup == "" { + return trace.BadParameter("discovery group in discover_ec2.instances field is required") + } } return nil } + +// TaskNameForDiscoverEC2Parts are the fields that deterministically compute a Discover EC2 task name. +// To be used with TaskNameForDiscoverEC2 function. +type TaskNameForDiscoverEC2Parts struct { + Integration string + IssueType string + AccountID string + Region string +} + +// TaskNameForDiscoverEC2 returns a deterministic name for the DiscoverEC2 task type. +// This method is used to ensure a single UserTask is created to report issues in enrolling EC2 instances for a given integration, issue type, account id and region. +func TaskNameForDiscoverEC2(parts TaskNameForDiscoverEC2Parts) string { + var bs []byte + bs = append(bs, binary.LittleEndian.AppendUint64(nil, uint64(len(parts.Integration)))...) + bs = append(bs, []byte(parts.Integration)...) + bs = append(bs, binary.LittleEndian.AppendUint64(nil, uint64(len(parts.IssueType)))...) + bs = append(bs, []byte(parts.IssueType)...) + bs = append(bs, binary.LittleEndian.AppendUint64(nil, uint64(len(parts.AccountID)))...) + bs = append(bs, []byte(parts.AccountID)...) + bs = append(bs, binary.LittleEndian.AppendUint64(nil, uint64(len(parts.Region)))...) + bs = append(bs, []byte(parts.Region)...) + return uuid.NewSHA1(discoverEC2Namespace, bs).String() +} + +// discoverEC2Namespace is an UUID that represents the name space to be used for generating UUIDs for DiscoverEC2 User Task names. +var discoverEC2Namespace = uuid.Must(uuid.Parse("6ba7b815-9dad-11d1-80b4-00c04fd430c8")) diff --git a/api/types/usertasks/object_test.go b/api/types/usertasks/object_test.go index a8f4c6769ca82..3c6f77211a9ca 100644 --- a/api/types/usertasks/object_test.go +++ b/api/types/usertasks/object_test.go @@ -20,8 +20,10 @@ package usertasks_test import ( "testing" + "time" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/timestamppb" headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1" usertasksv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/usertasks/v1" @@ -31,39 +33,239 @@ import ( func TestValidateUserTask(t *testing.T) { t.Parallel() + exampleInstanceID := "i-123" + + baseEC2DiscoverTask := func(t *testing.T) *usertasksv1.UserTask { + userTask, err := usertasks.NewDiscoverEC2UserTask(&usertasksv1.UserTaskSpec{ + Integration: "my-integration", + TaskType: "discover-ec2", + IssueType: "ec2-eice-create-node", + State: "OPEN", + DiscoverEc2: &usertasksv1.DiscoverEC2{ + AccountId: "123456789012", + Region: "us-east-1", + Instances: map[string]*usertasksv1.DiscoverEC2Instance{ + exampleInstanceID: { + InstanceId: exampleInstanceID, + DiscoveryConfig: "dc01", + DiscoveryGroup: "dg01", + SyncTime: timestamppb.Now(), + }, + }, + }, + }) + require.NoError(t, err) + return userTask + } + tests := []struct { name string - task *usertasksv1.UserTask + task func(t *testing.T) *usertasksv1.UserTask wantErr require.ErrorAssertionFunc }{ { - name: "NilUserTask", - task: nil, + name: "nil user task", + task: func(t *testing.T) *usertasksv1.UserTask { + return nil + }, + wantErr: require.Error, + }, + { + name: "invalid task type", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.TaskType = "invalid" + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: valid", + task: baseEC2DiscoverTask, + wantErr: require.NoError, + }, + { + name: "DiscoverEC2: invalid state", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.State = "invalid" + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: invalid issue type", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.IssueType = "unknown error" + return ut + }, wantErr: require.Error, }, { - name: "ValidUserTask", - task: &usertasksv1.UserTask{ + name: "DiscoverEC2: missing integration", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.Integration = "" + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: missing discover ec2 field", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.DiscoverEc2 = nil + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: wrong task name", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Metadata.Name = "another-name" + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: missing account id", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.DiscoverEc2.AccountId = "" + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: missing region", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + ut.Spec.DiscoverEc2.Region = "" + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: instances - missing instance id in map key", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + origInstanceMetadata := ut.Spec.DiscoverEc2.Instances[exampleInstanceID] + ut.Spec.DiscoverEc2.Instances[""] = origInstanceMetadata + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: instances - missing instance id in instance metadata", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + origInstanceMetadata := ut.Spec.DiscoverEc2.Instances[exampleInstanceID] + origInstanceMetadata.InstanceId = "" + ut.Spec.DiscoverEc2.Instances[exampleInstanceID] = origInstanceMetadata + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: instances - different instance id", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + origInstanceMetadata := ut.Spec.DiscoverEc2.Instances[exampleInstanceID] + origInstanceMetadata.InstanceId = "i-000" + ut.Spec.DiscoverEc2.Instances[exampleInstanceID] = origInstanceMetadata + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: instances - missing discovery config", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + origInstanceMetadata := ut.Spec.DiscoverEc2.Instances[exampleInstanceID] + origInstanceMetadata.DiscoveryConfig = "" + ut.Spec.DiscoverEc2.Instances[exampleInstanceID] = origInstanceMetadata + return ut + }, + wantErr: require.Error, + }, + { + name: "DiscoverEC2: instances - missing discovery group", + task: func(t *testing.T) *usertasksv1.UserTask { + ut := baseEC2DiscoverTask(t) + origInstanceMetadata := ut.Spec.DiscoverEc2.Instances[exampleInstanceID] + origInstanceMetadata.DiscoveryGroup = "" + ut.Spec.DiscoverEc2.Instances[exampleInstanceID] = origInstanceMetadata + return ut + }, + wantErr: require.Error, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := usertasks.ValidateUserTask(tt.task(t)) + tt.wantErr(t, err) + }) + } +} + +func TestNewDiscoverEC2UserTask(t *testing.T) { + t.Parallel() + + userTaskExpirationTime := time.Now() + userTaskExpirationTimestamp := timestamppb.New(userTaskExpirationTime) + instanceSyncTimestamp := userTaskExpirationTimestamp + + baseEC2DiscoverTaskSpec := &usertasksv1.UserTaskSpec{ + Integration: "my-integration", + TaskType: "discover-ec2", + IssueType: "ec2-eice-create-node", + State: "OPEN", + DiscoverEc2: &usertasksv1.DiscoverEC2{ + AccountId: "123456789012", + Region: "us-east-1", + Instances: map[string]*usertasksv1.DiscoverEC2Instance{ + "i-123": { + InstanceId: "i-123", + DiscoveryConfig: "dc01", + DiscoveryGroup: "dg01", + SyncTime: instanceSyncTimestamp, + }, + }, + }, + } + + tests := []struct { + name string + taskSpec *usertasksv1.UserTaskSpec + taskOption []usertasks.UserTaskOption + expectedTask *usertasksv1.UserTask + }{ + { + name: "options are applied task type", + taskSpec: baseEC2DiscoverTaskSpec, + expectedTask: &usertasksv1.UserTask{ Kind: "user_task", Version: "v1", Metadata: &headerv1.Metadata{ - Name: "test", - }, - Spec: &usertasksv1.UserTaskSpec{ - Integration: "my-integration", - TaskType: "discover-ec2", - IssueType: "failed to enroll ec2 instances", - DiscoverEc2: &usertasksv1.DiscoverEC2{}, + Name: "bd1e9ec3-33d3-52f8-a674-c8145e739559", + Expires: userTaskExpirationTimestamp, }, + Spec: baseEC2DiscoverTaskSpec, + }, + taskOption: []usertasks.UserTaskOption{ + usertasks.WithExpiration(userTaskExpirationTime), }, - wantErr: require.NoError, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := usertasks.ValidateUserTask(tt.task) - tt.wantErr(t, err) + gotTask, err := usertasks.NewDiscoverEC2UserTask(tt.taskSpec, tt.taskOption...) + require.NoError(t, err) + require.Equal(t, tt.expectedTask, gotTask) }) } } diff --git a/lib/cache/cache_test.go b/lib/cache/cache_test.go index 70fa65e305f75..13849f4855ccf 100644 --- a/lib/cache/cache_test.go +++ b/lib/cache/cache_test.go @@ -38,6 +38,7 @@ import ( "github.com/stretchr/testify/require" protobuf "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" + "google.golang.org/protobuf/types/known/timestamppb" "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" @@ -2299,7 +2300,7 @@ func TestUserTasks(t *testing.T) { testResources153(t, p, testFuncs153[*usertasksv1.UserTask]{ newResource: func(name string) (*usertasksv1.UserTask, error) { - return newUserTasks(t, name), nil + return newUserTasks(t), nil }, create: func(ctx context.Context, item *usertasksv1.UserTask) error { _, err := p.userTasks.CreateUserTask(ctx, item) @@ -2317,22 +2318,30 @@ func TestUserTasks(t *testing.T) { }) } -func newUserTasks(t *testing.T, name string) *usertasksv1.UserTask { +func newUserTasks(t *testing.T) *usertasksv1.UserTask { t.Helper() - return &usertasksv1.UserTask{ - Kind: types.KindUserTask, - Version: types.V1, - Metadata: &headerv1.Metadata{ - Name: name, - }, - Spec: &usertasksv1.UserTaskSpec{ - Integration: "my-integration", - TaskType: usertasks.TaskTypeDiscoverEC2, - IssueType: "my-issue-type", - DiscoverEc2: &usertasksv1.DiscoverEC2{}, + ut, err := usertasks.NewDiscoverEC2UserTask(&usertasksv1.UserTaskSpec{ + Integration: "my-integration", + TaskType: usertasks.TaskTypeDiscoverEC2, + IssueType: "ec2-ssm-agent-not-registered", + State: "OPEN", + DiscoverEc2: &usertasksv1.DiscoverEC2{ + AccountId: "123456789012", + Region: "us-east-1", + Instances: map[string]*usertasksv1.DiscoverEC2Instance{ + "i-123": { + InstanceId: "i-123", + DiscoveryConfig: "dc01", + DiscoveryGroup: "dg01", + SyncTime: timestamppb.Now(), + }, + }, }, - } + }) + require.NoError(t, err) + + return ut } // TestDiscoveryConfig tests that CRUD operations on DiscoveryConfig resources are @@ -3442,7 +3451,7 @@ func TestCacheWatchKindExistsInEvents(t *testing.T) { types.KindSPIFFEFederation: types.Resource153ToLegacy(newSPIFFEFederation("test")), types.KindAccessGraphSettings: types.Resource153ToLegacy(newAccessGraphSettings(t)), types.KindStaticHostUser: types.Resource153ToLegacy(newStaticHostUser(t, "test")), - types.KindUserTask: types.Resource153ToLegacy(newUserTasks(t, "test")), + types.KindUserTask: types.Resource153ToLegacy(newUserTasks(t)), types.KindAutoUpdateConfig: types.Resource153ToLegacy(newAutoUpdateConfig(t)), types.KindAutoUpdateVersion: types.Resource153ToLegacy(newAutoUpdateVersion(t)), } diff --git a/lib/services/local/user_task_test.go b/lib/services/local/user_task_test.go index e051c8d7456db..4d477bf166051 100644 --- a/lib/services/local/user_task_test.go +++ b/lib/services/local/user_task_test.go @@ -21,6 +21,8 @@ package local_test import ( "context" "fmt" + "slices" + "strings" "testing" "time" @@ -45,13 +47,7 @@ func TestCreateUserTask(t *testing.T) { ctx := context.Background() service := getUserTasksService(t) - obj, err := usertasks.NewUserTask("obj", &usertasksv1.UserTaskSpec{ - Integration: "my-integration", - TaskType: "discover-ec2", - IssueType: "ssm_agent_not_running", - DiscoverEc2: &usertasksv1.DiscoverEC2{}, - }) - require.NoError(t, err) + obj := getUserTaskObject(t, 0) // first attempt should succeed objOut, err := service.CreateUserTask(ctx, obj) @@ -68,13 +64,7 @@ func TestUpsertUserTask(t *testing.T) { ctx := context.Background() service := getUserTasksService(t) - obj, err := usertasks.NewUserTask("obj", &usertasksv1.UserTaskSpec{ - Integration: "my-integration", - TaskType: "discover-ec2", - IssueType: "ssm_agent_not_running", - DiscoverEc2: &usertasksv1.DiscoverEC2{}, - }) - require.NoError(t, err) + obj := getUserTaskObject(t, 0) // the first attempt should succeed objOut, err := service.UpsertUserTask(ctx, obj) require.NoError(t, err) @@ -215,12 +205,25 @@ func TestListUserTask(t *testing.T) { ctx := context.Background() + cmpOpts := []cmp.Option{ + protocmp.IgnoreFields(&headerv1.Metadata{}, "revision"), + protocmp.Transform(), + } + sortUserTasksFn := func(a *usertasksv1.UserTask, b *usertasksv1.UserTask) int { + return strings.Compare(a.Metadata.GetName(), b.Metadata.GetName()) + } counts := []int{0, 1, 5, 10} for _, count := range counts { t.Run(fmt.Sprintf("count=%v", count), func(t *testing.T) { service := getUserTasksService(t) prepopulateUserTask(t, service, count) + expectedElements := make([]*usertasksv1.UserTask, 0, count) + for i := 0; i < count; i++ { + expectedElements = append(expectedElements, getUserTaskObject(t, i)) + } + slices.SortFunc(expectedElements, sortUserTasksFn) + t.Run("one page", func(t *testing.T) { // Fetch all objects. elements, nextToken, err := service.ListUserTasks(ctx, 200, "") @@ -228,13 +231,8 @@ func TestListUserTask(t *testing.T) { require.Empty(t, nextToken) require.Len(t, elements, count) - for i := 0; i < count; i++ { - cmpOpts := []cmp.Option{ - protocmp.IgnoreFields(&headerv1.Metadata{}, "revision"), - protocmp.Transform(), - } - require.Equal(t, "", cmp.Diff(getUserTaskObject(t, i), elements[i], cmpOpts...)) - } + slices.SortFunc(elements, sortUserTasksFn) + require.Equal(t, "", cmp.Diff(expectedElements, elements, cmpOpts...)) }) t.Run("paginated", func(t *testing.T) { @@ -252,13 +250,9 @@ func TestListUserTask(t *testing.T) { } } - for i := 0; i < count; i++ { - cmpOpts := []cmp.Option{ - protocmp.IgnoreFields(&headerv1.Metadata{}, "revision"), - protocmp.Transform(), - } - require.Equal(t, "", cmp.Diff(getUserTaskObject(t, i), elements[i], cmpOpts...)) - } + require.Len(t, expectedElements, len(elements)) + slices.SortFunc(elements, sortUserTasksFn) + require.Equal(t, "", cmp.Diff(expectedElements, elements, cmpOpts...)) }) }) } @@ -277,15 +271,27 @@ func getUserTasksService(t *testing.T) services.UserTasks { } func getUserTaskObject(t *testing.T, index int) *usertasksv1.UserTask { - name := fmt.Sprintf("obj%v", index) - obj, err := usertasks.NewUserTask(name, &usertasksv1.UserTaskSpec{ - Integration: "my-integration", + integrationName := fmt.Sprintf("integration-%d", index) + + obj, err := usertasks.NewDiscoverEC2UserTask(&usertasksv1.UserTaskSpec{ + Integration: integrationName, TaskType: "discover-ec2", - IssueType: "ssm_agent_not_running", - DiscoverEc2: &usertasksv1.DiscoverEC2{}, + IssueType: "ec2-ssm-agent-not-registered", + State: "OPEN", + DiscoverEc2: &usertasksv1.DiscoverEC2{ + AccountId: "123456789012", + Region: "us-east-1", + Instances: map[string]*usertasksv1.DiscoverEC2Instance{ + "i-123": { + InstanceId: "i-123", + Name: "my-instance", + DiscoveryConfig: "dc01", + DiscoveryGroup: "dg01", + }, + }, + }, }) require.NoError(t, err) - require.NoError(t, err) return obj } diff --git a/lib/services/user_task_test.go b/lib/services/user_task_test.go index 46be4b7c209a6..b11a5e27f535f 100644 --- a/lib/services/user_task_test.go +++ b/lib/services/user_task_test.go @@ -49,16 +49,19 @@ func TestMarshalUserTaskRoundTrip(t *testing.T) { TaskType: "discover-ec2", IssueType: "SSM_AGENT_MISSING", State: "OPEN", - DiscoverEc2: &usertasksv1.DiscoverEC2{Instances: map[string]*usertasksv1.DiscoverEC2Instance{ - "i-1234567890": { - Name: "instance-name", - Region: "us-east-1", - InvocationUrl: "https://example.com/", - DiscoveryConfig: "config", - DiscoveryGroup: "group", - SyncTime: timestamppb.Now(), + DiscoverEc2: &usertasksv1.DiscoverEC2{ + Region: "us-east-1", + AccountId: "123456789012", + Instances: map[string]*usertasksv1.DiscoverEC2Instance{ + "i-1234567890": { + Name: "instance-name", + InvocationUrl: "https://example.com/", + DiscoveryConfig: "config", + DiscoveryGroup: "group", + SyncTime: timestamppb.Now(), + }, }, - }}, + }, }, } @@ -88,10 +91,11 @@ spec: issue_type: SSM_AGENT_MISSING state: OPEN discover_ec2: + region: us-east-1 + account_id: "123456789012" instances: i-1234567890: name: instance-name - region: us-east-1 invocation_url: https://example.com/ discovery_config: config discovery_group: group @@ -115,16 +119,19 @@ spec: TaskType: "discover-ec2", IssueType: "SSM_AGENT_MISSING", State: "OPEN", - DiscoverEc2: &usertasksv1.DiscoverEC2{Instances: map[string]*usertasksv1.DiscoverEC2Instance{ - "i-1234567890": { - Name: "instance-name", - Region: "us-east-1", - InvocationUrl: "https://example.com/", - DiscoveryConfig: "config", - DiscoveryGroup: "group", - SyncTime: syncTime, + DiscoverEc2: &usertasksv1.DiscoverEC2{ + Region: "us-east-1", + AccountId: "123456789012", + Instances: map[string]*usertasksv1.DiscoverEC2Instance{ + "i-1234567890": { + Name: "instance-name", + InvocationUrl: "https://example.com/", + DiscoveryConfig: "config", + DiscoveryGroup: "group", + SyncTime: syncTime, + }, }, - }}, + }, }, }