Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add opensearch packages vpc endpoint support #1078

Conversation

swhite-oreilly
Copy link
Contributor

@swhite-oreilly swhite-oreilly commented Aug 18, 2023

This PR includes two new modules for Opensearch Packages and VPC Endpoints.

Testing

Opensearch packages and VPC endpoints were created. using the setup code mentioned below, and then AWS Nuke was used to clean these resources up, specifying OSPackage and OSVPCEndpoint.

us-east-1 - OSVPCEndpoint - aos-a50c4cac2357 - [VpcEndpointId: "aos-a50c4cac2357"] - would remove
us-east-1 - OSDomain - test-domain - [LastUpdatedTime: "2023-08-24T13:45:54Z"] - would remove
Scan complete: 2 total, 2 nukeable, 0 filtered.

Setup code

# Create an OpenSearch domain
echo "Creating OpenSearch domain"
DOMAIN=`aws opensearch create-domain --domain-name mylogs --engine-version OpenSearch_1.2 --cluster-config  InstanceType=t3.small.search,InstanceCount=1 --ebs-options EBSEnabled=true,VolumeType=gp3,VolumeSize=100,Iops=3500,Throughput=125 --access-policies '{"Version": "2012-10-17", "Statement": [{"Action": "es:*", "Principal":"*","Effect": "Allow", "Condition": {"IpAddress":{"aws:SourceIp":["192.0.2.0/32"]}}}]}'`

# Extract the domain name using grep and awk
DOMAIN_NAME=$(echo "$DOMAIN" | grep -o '"DomainName": "[^"]*' | awk -F'"' '{print $4}')

# Print the extracted domain name
echo "The domain name is: $DOMAIN_NAME"

# Extract the ARN using grep and awk
DOMAIN_ARN=$(echo "$DOMAIN" | grep -o '"ARN": "[^"]*' | awk -F'"' '{print $4}')
echo "The domain ARN is: $DOMAIN_ARN"

# Create a file to upload to S3
echo "Generating file to upload to S3 for OpenSearch custom package"
cat <<EOF > synonyms.txt
danish, croissant, pastry
ice cream, gelato, frozen custard
sneaker, tennis shoe, running shoe
basketball shoe, hightop
EOF

# Generate a random string to use as a bucket name
RANDOM_STRING=$(openssl rand -hex 20)

# Create a S3 Bucket to store the file:
echo "Creating S3 bucket to store OpenSearch custom package file"
aws s3api create-bucket --bucket opensearch-packages-$RANDOM_STRING --region us-east-1

# Copy the provided generated synonym to the S3 bucket:
echo "Uploading file to S3 bucket"
aws s3 cp ./synonyms.txt s3://opensearch-packages-$RANDOM_STRING

# Creating an OpenSearch custom package (Note: This command will fail if the bucket and file do not exist)
echo "Creating OpenSearch custom package"
aws opensearch create-package --package-name test-package --package-type 'TXT-DICTIONARY' --package-source '{"S3BucketName": "'"opensearch-packages-$RANDOM_STRING"'", "S3Key": "synonyms.txt"}'


echo "Getting a subnet from the default VPC"
DEFAULT_VPC_ID=$(aws ec2 describe-vpcs --filters "Name=isDefault,Values=true" --query 'Vpcs[0].VpcId' --output text)

SUBNET_ID=$(aws ec2 describe-subnets --filters "Name=vpc-id,Values=$DEFAULT_VPC_ID" "Name=availability-zone,Values=us-east-1*" --query 'Subnets[0].SubnetId' --output text)
echo "Using subnet $SUBNET_ID"

# Get the security group ID
SECURITY_GROUP_ID=$(aws ec2 describe-security-groups --filters "Name=vpc-id,Values=$DEFAULT_VPC_ID" --query 'SecurityGroups[0].GroupId' --output text)
echo "Using security group $SECURITY_GROUP_ID"

# Create an Opensearch VPC Endpoint (Note: Using the default VPC appears to work intermittently, but creating a new VPC and then creating the VPC endpoint works consistently)
echo "Creating OpenSearch VPC Endpoint"
VPC_OPTIONS='{"SubnetIds": ["'"$SUBNET_ID"'"], "SecurityGroupIds": ["'"$SECURITY_GROUP_ID"'"]}'
echo "VPC options: $VPC_OPTIONS"

# Create the VPC endpoint
aws opensearch create-vpc-endpoint \
  --domain-arn "$DOMAIN_ARN" \
  --vpc-options "$VPC_OPTIONS"

Adding support for opensearch packages.
Confirmed working cleanup of os packages.
Removing unused var from packages.
Correctly retrieving VPC endpoint ids.
Setting property values.
@swhite-oreilly swhite-oreilly requested a review from a team as a code owner August 18, 2023 22:03
Comment on lines 21 to 40
vpcEndpointIds, err := getOpenSearchVpcEndpointIds(svc)
if err != nil {
return nil, err
}

listResp, err := svc.DescribeVpcEndpoints(&opensearchservice.DescribeVpcEndpointsInput{
VpcEndpointIds: vpcEndpointIds,
})
if err != nil {
return nil, err
}

resources := make([]Resource, 0)

for _, vpcEndpoint := range listResp.VpcEndpoints {
resources = append(resources, &OSVPCEndpoint{
svc: svc,
vpcEndpointId: vpcEndpoint.VpcEndpointId,
})
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!
If we have the IDs already, why are we describing them again? To make sure they exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@der-eismann apologies. You are correct. That should have been removed and is now updated.

Comment on lines 25 to 41
listResp, err := svc.DescribePackages(&opensearchservice.DescribePackagesInput{})
if err != nil {
return nil, err
}

resources := make([]Resource, 0)

for _, pkg := range listResp.PackageDetailsList {
resources = append(resources, &OSPackage{
svc: svc,
packageID: pkg.PackageID,
packageName: pkg.PackageName,
createdTime: pkg.CreatedAt,
})
}

return resources, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, another thing I just noticed - both resources need paging as well, just like you did in #1044. Could you do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@der-eismann No worries, updated with pagination and noted for my next PR's!

Comment on lines 18 to 51
func ListOSVPCEndpoints(sess *session.Session) ([]Resource, error) {
svc := opensearchservice.New(sess)

vpcEndpointIds, err := getOpenSearchVpcEndpointIds(svc)
if err != nil {
return nil, err
}

resources := make([]Resource, 0)

for _, vpcEndpointId := range vpcEndpointIds {
resources = append(resources, &OSVPCEndpoint{
svc: svc,
vpcEndpointId: vpcEndpointId,
})
}

return resources, nil
}

func getOpenSearchVpcEndpointIds(svc *opensearchservice.OpenSearchService) ([]*string, error) {
vpcEndpointIds := make([]*string, 0)

listResp, err := svc.ListVpcEndpoints(&opensearchservice.ListVpcEndpointsInput{})
if err != nil {
return nil, err
}

for _, vpcEndpoint := range listResp.VpcEndpointSummaryList {
vpcEndpointIds = append(vpcEndpointIds, vpcEndpoint.VpcEndpointId)
}

return vpcEndpointIds, nil
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't really get why you need that extra func with an extra loop in it. I think it makes more sense to do it just like you did for the OS Packages. Just list the endpoints and add them directly to the []Resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@der-eismann I have cleaned this up to match the OS Packages update.

@der-eismann der-eismann merged commit 5342982 into rebuy-de:main Aug 25, 2023
2 checks passed
@der-eismann
Copy link
Member

Hi @swhite-oreilly, we are suddenly seeing errors in the form of:

eu-west-1 - OSPackage - G234201692 - [CreatedTime: "2023-10-12T17:18:00Z", PackageID: "G234201692", PackageName: "analysis-pinyin"] - failed
eu-west-1 - OSPackage - G253805861 - [CreatedTime: "2023-10-12T17:17:03Z", PackageID: "G253805861", PackageName: "analysis-nori"] - failed
eu-west-1 - OSPackage - G256061839 - [CreatedTime: "2023-10-12T17:24:36Z", PackageID: "G256061839", PackageName: "analysis-sudachi"] - failed
eu-west-1 - OSPackage - G262703391 - [CreatedTime: "2023-10-12T17:22:44Z", PackageID: "G262703391", PackageName: "analysis-stconvert"] - failed
eu-west-1 - OSPackage - G29550247 - [CreatedTime: "2023-10-12T17:17:07Z", PackageID: "G29550247", PackageName: "analysis-nori"] - failed
eu-west-1 - OSPackage - G38447777 - [CreatedTime: "2023-10-12T17:18:18Z", PackageID: "G38447777", PackageName: "analysis-stconvert"] - failed
time="2023-10-16T20:01:59Z" level=error msg="ValidationException: Operation not allowed. This package cannot be deleted."
time="2023-10-16T20:01:59Z" level=error msg="ValidationException: Operation not allowed. This package cannot be deleted."
time="2023-10-16T20:01:59Z" level=error msg="ValidationException: Operation not allowed. This package cannot be deleted."
time="2023-10-16T20:01:59Z" level=error msg="ValidationException: Operation not allowed. This package cannot be deleted."
time="2023-10-16T20:01:59Z" level=error msg="ValidationException: Operation not allowed. This package cannot be deleted."
time="2023-10-16T20:01:59Z" level=error msg="ValidationException: Operation not allowed. This package cannot be deleted."

We are not working with OpenSearch von AWS, so I'm not really sure what changed for the error to occur. Do we need to filter these packages?

@swhite-oreilly
Copy link
Contributor Author

@der-eismann, we are experiencing this as well on our end. It appears AWS added some default packages and they can't be deleted. I think filtering will need to be added.

@swhite-oreilly
Copy link
Contributor Author

@der-eismann I just noticed a new one called amazon-personalized-ranking. I will have a filter PR up shortly. I hope this doesn't turn into whack-a-mole.

@der-eismann
Copy link
Member

I'm unsure if that filter should be name based, because then it will break again and again if they add new ones. On the other hand I see no way to differentiate AWS-provided packages from your own.

@swhite-oreilly
Copy link
Contributor Author

@der-eismann open to suggestions/recommendations. I am hoping there is a limit to how many packages AWS adds for all users.

@swhite-oreilly
Copy link
Contributor Author

swhite-oreilly commented Oct 18, 2023

@der-eismann one of my team members noticed that the ID's for all the internal packages start with G while user created packages appear to start with F. We have a support ticket in with AWS to verify. If that is the case, I will update the filter and test before submitting a PR.

@swhite-oreilly
Copy link
Contributor Author

@der-eismann I have a PR up to fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants