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.

Sign up for GitHub

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

Jump to bottom

Teleport install command for discovery agents (go) #43423

Merged

marcoandredinis merged 4 commits into master from marco/server-autodiscover-script-go

Jul 9, 2024

Contributor

marcoandredinis commented Jun 24, 2024

This PR adds a new command that will install teleport on the current system.
Its goal is to replace the default installer script used in Auto Discovery scripts.

teleport/api/types/installers/installer.sh.tmpl

Line 1 in 3d8fb11

#!/usr/bin/env bash

Its correctness should be compared to the current script
https://github.com/gravitational/teleport/blob/master/api/types/installers/installer.sh.tmpl
Improvements will only be considered on a coming PR.

marcoandredinis added no-changelog backport/branch/v15 backport/branch/v16 labels

github-actions bot requested review from kimlisa and lxea

June 24, 2024 17:32

public-teleport-github-review-bot bot commented Jun 24, 2024

@marcoandredinis - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

github-actions bot added the size/xl label

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch from aadb50e to 4466762 Compare

June 25, 2024 07:52

marcoandredinis mentioned this pull request

Improve robustness and auditing of discovery service #37620

Open

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch 5 times, most recently from 6281e59 to 0cdf101 Compare

June 25, 2024 15:35

marcoandredinis requested a review from r0mant

June 25, 2024 16:51

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch from 0cdf101 to 7e35c38 Compare

June 28, 2024 13:14

r0mant approved these changes

View reviewed changes

Collaborator

r0mant left a comment

Looks pretty good to me, just left a few suggestions about reuse/organization.

lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved

lib/srv/server/installer/autodiscover.go Outdated

Comment on lines 316 to 209

    
              	// Check if teleport is already installed.

              	if _, err := os.Stat(asi.binariesLocation.teleport); err == nil {

              		asi.Logger.InfoContext(ctx, "Teleport is already installed in the system.")

              		return nil

              	}

Collaborator

r0mant Jul 1, 2024

Can it happen that teleport binary is present but we failed to generate/write config (or it got removed for some reason)? In this case the method will not proceed but installation will not be valid.

Or are you planning to address this scenario in a follow-up PR?

Contributor Author

marcoandredinis Jul 3, 2024

Yes, I'm planning on multiple follow-up PRs
I'm tracking improvements on the 1st tasklist of this issue #37620

Feel free to add more improvements there 👍

lib/srv/server/installer/autodiscover.go

    
              		return trace.Wrap(err)

              	}

              	// The last step is to configure the `teleport.yaml`.

Collaborator

r0mant Jul 1, 2024

Minor-ish refactoring request. This function does 2 things: installs teleport and creates its configuration. Can these 2 things be factored out into corresponding methods (installTeleportPackage, configureTeleport)?

Contributor Author

marcoandredinis Jul 3, 2024

Yes 👍

lib/srv/server/installer/autodiscover.go Outdated

    
              	return imdsClient, nil

              }

              func (asi *AutodiscoverNodeInstaller) fetchTargetVersion(ctx context.Context) string {

Collaborator

r0mant Jul 1, 2024

We have the logic for dealing with automatic updates versions in lib/automaticupgrades/channel.go. Is this something that we can reuse (or possibly extend) instead of reimplementing it here?

Contributor Author

marcoandredinis Jul 3, 2024

We already use the github.com/gravitational/teleport/lib/automaticupgrades/version package.

Aside from parsing the URL from string into net.URL we would call this method:

	targetVersion, err := version.NewBasicHTTPVersionGetter(upgradeURL).GetVersion(ctx)

@hugoShaka Is there something else we can add to the package?

lib/srv/server/installer/autodiscover.go

    
              	return strings.TrimSpace(strings.TrimPrefix(targetVersion, "v"))

              }

              func fetchNodeAutoDiscoverLabels(ctx context.Context, imdsClient imds.Client) (map[string]string, error) {

Collaborator

r0mant Jul 1, 2024

I think we already automatically fetch node labels on every cloud we support. Is this something we can reuse? cc @atburke would probably know.

Contributor Author

marcoandredinis Jul 3, 2024

For the record, this is the equivalent in the current shell script

teleport/api/types/installers/installer.sh.tmpl

Lines 151 to 183 in d9bcd2e

    
           if on_azure; then 
        
             API_VERSION=$(curl -m5 -sS -H "Metadata: true" --noproxy "*" "http://169.254.169.254/metadata/versions" | jq -r ".apiVersions[-1]") 
        
             INSTANCE_INFO=$(curl -m5 -sS -H "Metadata: true" --noproxy "*" "http://169.254.169.254/metadata/instance?api-version=$API_VERSION&format=json") 
        
             REGION="$(echo "$INSTANCE_INFO" | jq -r .compute.location)" 
        
             RESOURCE_GROUP="$(echo "$INSTANCE_INFO" | jq -r .compute.resourceGroupName)" 
        
             SUBSCRIPTION_ID="$(echo "$INSTANCE_INFO" | jq -r .compute.subscriptionId)" 
        
             VM_ID="$(echo "$INSTANCE_INFO" | jq -r .compute.vmId)" 
        
             JOIN_METHOD=azure 
        
             LABELS="teleport.internal/vm-id=${VM_ID},teleport.internal/subscription-id=${SUBSCRIPTION_ID},teleport.internal/region=${REGION},teleport.internal/resource-group=${RESOURCE_GROUP}" 
        
           elif on_ec2; then 
        
             IMDS_TOKEN=$(curl -m5 -sS -X PUT "http://169.254.169.254/latest/api/token" -H "X-aws-ec2-metadata-token-ttl-seconds: 300") 
        
             INSTANCE_INFO=$(curl -m5 -sS -H "X-aws-ec2-metadata-token: ${IMDS_TOKEN}" "http://169.254.169.254/latest/dynamic/instance-identity/document") 
        
             ACCOUNT_ID="$(echo "$INSTANCE_INFO" | jq -r .accountId)" 
        
             INSTANCE_ID="$(echo "$INSTANCE_INFO" | jq -r .instanceId)" 
        
             JOIN_METHOD=iam 
        
             LABELS="teleport.dev/instance-id=${INSTANCE_ID},teleport.dev/account-id=${ACCOUNT_ID}" 
        
           elif on_gcp; then 
        
             NAME="$(curl -m5 -sS -H "Metadata-Flavor:Google" "http://metadata.google.internal/computeMetadata/v1/instance/name")" 
        
             # GCP metadata returns fully qualified zone ("projects/<project-id>/zones/<zone>"), so we need to parse the zone name. 
        
             FULL_ZONE="$(curl -m5 -sS -H "Metadata-Flavor:Google" "http://metadata.google.internal/computeMetadata/v1/instance/zone")" 
        
             ZONE="$(basename $FULL_ZONE)" 
        
             PROJECT_ID=$(curl -m5 -sS -H "Metadata-Flavor: Google" "http://metadata.google.internal/computeMetadata/v1/project/project-id") 
        
             JOIN_METHOD=gcp 
        
             LABELS="teleport.internal/name=${NAME},teleport.internal/zone=${ZONE},teleport.internal/project-id=${PROJECT_ID}" 
        
           else 
        
             echo "Could not determine cloud provider" 
        
             exit 1 
        
           fi

These labels are not sourced from the instance's labels/tags. These are mostly metadata used for detecting already enrolled instances and some other metadata.

Let's use Azure as an example.
The script adds the following labels:

teleport/api/types/installers/installer.sh.tmpl

Lines 155 to 161 in d9bcd2e

    
           REGION="$(echo "$INSTANCE_INFO" | jq -r .compute.location)" 
        
           RESOURCE_GROUP="$(echo "$INSTANCE_INFO" | jq -r .compute.resourceGroupName)" 
        
           SUBSCRIPTION_ID="$(echo "$INSTANCE_INFO" | jq -r .compute.subscriptionId)" 
        
           VM_ID="$(echo "$INSTANCE_INFO" | jq -r .compute.vmId)" 
        
           JOIN_METHOD=azure 
        
           LABELS="teleport.internal/vm-id=${VM_ID},teleport.internal/subscription-id=${SUBSCRIPTION_ID},teleport.internal/region=${REGION},teleport.internal/resource-group=${RESOURCE_GROUP}"

The following labels are used to detect if the instance is already enrolled:

teleport/lib/srv/discovery/discovery.go

Lines 1046 to 1049 in 8410d1e

    
           labels := n.GetAllLabels() 
        
           _, subscriptionOK := labels[types.SubscriptionIDLabel] 
        
           _, vmOK := labels[types.VMIDLabel] 
        
           return subscriptionOK && vmOK

So, we end up with a couple more labels which might be used for RBAC.
Some of those labels are not present in any go code, and that's why I ended up adding them here
https://github.com/gravitational/teleport/blob/7e35c38679a71601234305baffa3cfa83c5b5fa3/api/types/constants.go#L675-L682

@atburke Can you please double check this?

Contributor

atburke Jul 8, 2024

Yep, that's all correct.

lib/srv/server/installer/autodiscover.go Outdated

    
              	*linux.OSRelease

              }

              func (l *linuxDistroInfo) packageManagerKind() packageManagerKind {

Collaborator

r0mant Jul 1, 2024

Nit: What do you think about moving this (along with all related logic/structs/constants) somewhere to utils package? Feels like this could be useful outside of just installer in future e.g. for the next-gen auto-updates work.

Contributor Author

marcoandredinis Jul 3, 2024

Moved things to lib/utils/packagemanager/ and removed some not-required types/methods.

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch 2 times, most recently from 6ea8381 to 26b461a Compare

July 3, 2024 13:33

Contributor Author

marcoandredinis commented Jul 3, 2024

@kimlisa @lxea Can you please take a look when you get a chance?

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch 3 times, most recently from ed0aadb to 79f6551 Compare

July 8, 2024 09:33

kimlisa approved these changes

View reviewed changes

lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved

lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved

lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved

tool/teleport/common/teleport.go Outdated Show resolved Hide resolved

public-teleport-github-review-bot bot removed request for lxea

July 8, 2024 15:56

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch 2 times, most recently from 00b86d3 to ad519e9 Compare

July 9, 2024 09:54

marcoandredinis enabled auto-merge

July 9, 2024 09:56


          Teleport install command for discovery agents (go)

cedadac

This PR adds a new command that will install teleport on the current
system.
Its goal is to replace the default installer script used in Auto
Discovery scripts.

Its correctness should be compared to the current script. Improvements
will only be considered on a 2nd phase.

marcoandredinis added 3 commits

July 9, 2024 11:15


          rename ani/asi and re-organize install/configure sections

5f1db09


          create lib/utils/packagemanager package

66b9e04


          rename Autodiscover to AutoDiscover

731b3dc

marcoandredinis force-pushed the marco/server-autodiscover-script-go branch from ad519e9 to 731b3dc Compare

July 9, 2024 10:16

marcoandredinis added this pull request to the merge queue

Merged via the queue into master with commit fe4418f

38 checks passed

marcoandredinis deleted the marco/server-autodiscover-script-go branch

July 9, 2024 10:52

public-teleport-github-review-bot bot commented Jul 9, 2024

@marcoandredinis See the table below for backport results.

Branch	Result
branch/v15	Create PR
branch/v16	Create PR

This was referenced Jul 9, 2024

[v15] Teleport install command for discovery agents (go) #43947

Closed

[v16] Teleport install command for discovery agents (go) #43948

Merged

marcoandredinis removed the backport/branch/v15 label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v16 no-changelog size/xl