From fb3a46c8833efac9e1b7086d627f82f3e6b90fe4 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 1 Oct 2024 21:31:31 +0100 Subject: [PATCH 1/3] 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. --- .../go/teleport/usertasks/v1/user_tasks.pb.go | 74 +++--- .../teleport/usertasks/v1/user_tasks.proto | 12 +- api/types/autodiscover.go | 60 +++++ api/types/usertasks/object.go | 160 ++++++++++-- api/types/usertasks/object_test.go | 232 ++++++++++++++++-- lib/cache/cache_test.go | 39 +-- lib/services/local/user_task_test.go | 79 +++--- lib/services/user_task_test.go | 45 ++-- 8 files changed, 555 insertions(+), 146 deletions(-) create mode 100644 api/types/autodiscover.go 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 93718a7049a94..a35ab99640bee 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 this issue is happening. + 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..e3b183d2ac86f 100644 --- a/api/proto/teleport/usertasks/v1/user_tasks.proto +++ b/api/proto/teleport/usertasks/v1/user_tasks.proto @@ -63,6 +63,10 @@ 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 this issue is happening. + string region = 3; } // DiscoverEC2Instance contains the result of enrolling an AWS EC2 Instance. @@ -72,10 +76,6 @@ message DiscoverEC2Instance { // 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; @@ -85,4 +85,8 @@ message DiscoverEC2Instance { string discovery_group = 7; // SyncTime is the timestamp when the error was produced. google.protobuf.Timestamp sync_time = 8; + + // AccountID and Region were moved into the DiscoverEC2 message. + reserved 3, 4; + reserved "account_id", "region"; } diff --git a/api/types/autodiscover.go b/api/types/autodiscover.go new file mode 100644 index 0000000000000..29b2ac344f4d4 --- /dev/null +++ b/api/types/autodiscover.go @@ -0,0 +1,60 @@ +/* +Copyright 2024 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package types + +// 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 unlekly 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 unlekly 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 + // - 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" +) diff --git a/api/types/usertasks/object.go b/api/types/usertasks/object.go index 978d10c3d1121..e7f9381f34ca1 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,124 @@ const ( TaskTypeDiscoverEC2 = "discover-ec2" ) +// discoverEC2IssueTypes is a list of issue types that can occur when trying to auto enroll EC2 instances. +var discoverEC2IssueTypes = []string{ + types.AutoDiscoverEC2IssueEICEFailedToCreateNode, + types.AutoDiscoverEC2IssueScriptInstanceNotRegistered, + types.AutoDiscoverEC2IssueScriptInstanceConnectionLost, + types.AutoDiscoverEC2IssueScriptInstanceUnsupportedOS, + types.AutoDiscoverEC2IssueScriptFailure, +} + // 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 4292b93fb3b66..d633a2548ad5b 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" @@ -2276,7 +2277,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) @@ -2294,22 +2295,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 @@ -3421,7 +3430,7 @@ func TestCacheWatchKindExistsInEvents(t *testing.T) { types.KindStaticHostUser: types.Resource153ToLegacy(newStaticHostUser(t, "test")), types.KindAutoUpdateConfig: types.Resource153ToLegacy(newAutoUpdateConfig(t)), types.KindAutoUpdateVersion: types.Resource153ToLegacy(newAutoUpdateVersion(t)), - types.KindUserTask: types.Resource153ToLegacy(newUserTasks(t, "test")), + types.KindUserTask: types.Resource153ToLegacy(newUserTasks(t)), } for name, cfg := range cases { diff --git a/lib/services/local/user_task_test.go b/lib/services/local/user_task_test.go index 08bbdd98e3eae..8ad1b00361d7b 100644 --- a/lib/services/local/user_task_test.go +++ b/lib/services/local/user_task_test.go @@ -21,6 +21,7 @@ package local_test import ( "context" "fmt" + "slices" "testing" "time" @@ -44,13 +45,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) @@ -67,13 +62,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) @@ -214,12 +203,31 @@ 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 { + if a.Metadata.GetName() < b.Metadata.GetName() { + return 1 + } + if a.Metadata.GetName() > b.Metadata.GetName() { + return -1 + } + return 0 + } 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, "") @@ -227,13 +235,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) { @@ -251,13 +254,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...)) }) }) } @@ -276,15 +275,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, + }, }, - }}, + }, }, } From 24f6128f2136456abc2ce2cb64e7d38cf8a1ccd2 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Wed, 2 Oct 2024 14:23:09 +0100 Subject: [PATCH 2/3] use strings.Compare --- lib/services/local/user_task_test.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/services/local/user_task_test.go b/lib/services/local/user_task_test.go index 8ad1b00361d7b..48b1cf1c87c54 100644 --- a/lib/services/local/user_task_test.go +++ b/lib/services/local/user_task_test.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "slices" + "strings" "testing" "time" @@ -208,13 +209,7 @@ func TestListUserTask(t *testing.T) { protocmp.Transform(), } sortUserTasksFn := func(a *usertasksv1.UserTask, b *usertasksv1.UserTask) int { - if a.Metadata.GetName() < b.Metadata.GetName() { - return 1 - } - if a.Metadata.GetName() > b.Metadata.GetName() { - return -1 - } - return 0 + return strings.Compare(a.Metadata.GetName(), b.Metadata.GetName()) } counts := []int{0, 1, 5, 10} for _, count := range counts { From 43cbfc1ac0a27f31bc291ea049b1cf659872f6f0 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Fri, 4 Oct 2024 11:31:21 +0100 Subject: [PATCH 3/3] improve docs and fix typos --- .../go/teleport/usertasks/v1/user_tasks.pb.go | 2 +- .../teleport/usertasks/v1/user_tasks.proto | 10 ++-- api/types/autodiscover.go | 60 ------------------- api/types/usertasks/object.go | 56 +++++++++++++++-- 4 files changed, 57 insertions(+), 71 deletions(-) delete mode 100644 api/types/autodiscover.go 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 a35ab99640bee..2535a8c5c390d 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 @@ -226,7 +226,7 @@ type DiscoverEC2 struct { 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 this issue is happening. + // Region is the AWS Region where Teleport failed to enroll EC2 instances. Region string `protobuf:"bytes,3,opt,name=region,proto3" json:"region,omitempty"` } diff --git a/api/proto/teleport/usertasks/v1/user_tasks.proto b/api/proto/teleport/usertasks/v1/user_tasks.proto index e3b183d2ac86f..81d8d52d17b63 100644 --- a/api/proto/teleport/usertasks/v1/user_tasks.proto +++ b/api/proto/teleport/usertasks/v1/user_tasks.proto @@ -65,12 +65,16 @@ message DiscoverEC2 { map instances = 1; // AccountID is the AWS Account ID for the instances. string account_id = 2; - // Region is the AWS Region where this issue is happening. + // 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. @@ -85,8 +89,4 @@ message DiscoverEC2Instance { string discovery_group = 7; // SyncTime is the timestamp when the error was produced. google.protobuf.Timestamp sync_time = 8; - - // AccountID and Region were moved into the DiscoverEC2 message. - reserved 3, 4; - reserved "account_id", "region"; } diff --git a/api/types/autodiscover.go b/api/types/autodiscover.go deleted file mode 100644 index 29b2ac344f4d4..0000000000000 --- a/api/types/autodiscover.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -Copyright 2024 Gravitational, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package types - -// 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 unlekly 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 unlekly 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 - // - 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" -) diff --git a/api/types/usertasks/object.go b/api/types/usertasks/object.go index e7f9381f34ca1..df00f4e0a58a1 100644 --- a/api/types/usertasks/object.go +++ b/api/types/usertasks/object.go @@ -86,13 +86,59 @@ 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{ - types.AutoDiscoverEC2IssueEICEFailedToCreateNode, - types.AutoDiscoverEC2IssueScriptInstanceNotRegistered, - types.AutoDiscoverEC2IssueScriptInstanceConnectionLost, - types.AutoDiscoverEC2IssueScriptInstanceUnsupportedOS, - types.AutoDiscoverEC2IssueScriptFailure, + AutoDiscoverEC2IssueEICEFailedToCreateNode, + AutoDiscoverEC2IssueEICEFailedToUpsertNode, + AutoDiscoverEC2IssueScriptInstanceNotRegistered, + AutoDiscoverEC2IssueScriptInstanceConnectionLost, + AutoDiscoverEC2IssueScriptInstanceUnsupportedOS, + AutoDiscoverEC2IssueScriptFailure, + AutoDiscoverEC2IssueInvocationFailure, } // ValidateUserTask validates the UserTask object without modifying it.