From cc95c516e65b986739f45710a267b992e4cb96d2 Mon Sep 17 00:00:00 2001 From: DImuthuUpe Date: Sun, 25 Oct 2026 12:36:24 -0400 Subject: [PATCH] Refactoring ssh key API to support wide range of ssh credentials --- INSTALL.md | 17 +- api/compute/dto/config.go | 16 +- api/compute/model/cluster.go | 4 +- api/compute/repository/config.go | 6 +- api/compute/service/config.go | 28 +- api/compute/service/config_test.go | 20 +- api/credentials/controller/controller.go | 46 +-- .../dto/{ssh_key.go => ssh_credential.go} | 40 +-- .../model/{ssh_key.go => ssh_credential.go} | 42 +-- api/credentials/repository/repository.go | 40 +-- api/credentials/service/service.go | 81 ++--- api/data/dto/storage.go | 28 +- api/data/model/storage.go | 4 +- api/data/repository/storage.go | 8 +- api/data/service/storage.go | 26 +- api/data/service/transfer.go | 6 +- api/data/service/transport.go | 8 +- api/process/service/launch.go | 6 +- docs/api.md | 108 +++---- docs/openapi.yaml | 126 ++++---- docs/sharing.md | 2 +- internal/app/app.go | 16 +- internal/db/foreignkey_test.go | 6 +- internal/db/migrate.go | 2 +- internal/db/migrate_test.go | 2 +- .../migrations/0003_ssh_credential_rename.sql | 53 ++++ internal/httpx/errors.go | 2 +- internal/orchestration/batch.go | 15 +- internal/orchestration/data.go | 23 +- internal/orchestration/slurm.go | 3 +- internal/server/server.go | 2 +- internal/server/server_test.go | 130 ++++---- internal/ssh/operations.go | 123 ++++++++ internal/{orchestration => ssh}/ssh.go | 291 ++++++------------ internal/{orchestration => ssh}/ssh_test.go | 8 +- 35 files changed, 704 insertions(+), 634 deletions(-) rename api/credentials/dto/{ssh_key.go => ssh_credential.go} (65%) rename api/credentials/model/{ssh_key.go => ssh_credential.go} (57%) create mode 100644 internal/db/migrations/0003_ssh_credential_rename.sql create mode 100644 internal/ssh/operations.go rename internal/{orchestration => ssh}/ssh.go (73%) rename internal/{orchestration => ssh}/ssh_test.go (98%) diff --git a/INSTALL.md b/INSTALL.md index 66266cb8cb..eb084dc426 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -227,7 +227,7 @@ with no database. ## First run The root token authenticates as the principal `root` with the `SUPER_ADMIN` authority. -That is enough to administer clusters, SSH keys, templates and deployments right away: +That is enough to administer clusters, SSH credentials, templates and deployments right away: ```bash TOKEN='' @@ -243,7 +243,7 @@ curl -s -X POST localhost:9095/api/v1/slurm-clusters \ ### Owning resources requires a matching `users` row -Endpoints that create *owned* resources — SSH keys, cluster configs, data storages, +Endpoints that create *owned* resources — SSH credentials, cluster configs, data storages, processes — resolve the caller to a user record and refuse if none exists: ``` @@ -292,17 +292,18 @@ curl -s localhost:9095/api/v1/slurm-clusters # readable without a token ``` Catalogue reads — clusters, partitions, templates and deployments — are open to -anonymous callers. SSH keys are not: a key is private to whoever registered it. +anonymous callers. SSH credentials are not: a credential is private to whoever +registered it. Beyond those: -- **Writes** require `ADMIN` or `SUPER_ADMIN`, except creating SSH keys, cluster +- **Writes** require `ADMIN` or `SUPER_ADMIN`, except creating SSH credentials, cluster configs, data storages, data products and processes, which any authenticated caller may do for themselves. - **Owner-scoped reads** (`/slurm-cluster-configs/{id}`, `/users/{id}`) require authentication and are refused unless the caller owns the record, holds a share, or is - an admin. `/ssh-keys/{id}` is stricter still: the owner and nobody else, admins - included, because a key is the credential itself rather than something reached with - one. + an admin. `/ssh-credentials/{id}` is stricter still: the owner and nobody else, admins + included, because a credential *is* the secret material rather than something reached + with it. - **Unfiltered listings** of cluster configs, data storages, processes and users require `ADMIN`, because they expose who holds access to what. @@ -341,7 +342,7 @@ caller lacking the required role returns `403`. | `502 Unable to validate bearer token` | CILogon is unreachable. This is reported separately from `401` because it is not the caller's fault. | | `403 Access denied` on a write | The caller authenticated but lacks `ADMIN`/`SUPER_ADMIN`. Grant the role in `user_roles`. | | `404 No user record found for authenticated principal` | The caller has no `users` row. Register the user (see "First run" above). The `root` row is created automatically on startup. | -| `409 Key is in use by a credential` | An SSH key cannot be deleted while a credential references it. Delete the credential first. | +| `409 SSH credential is still presented by N record(s)` | A credential cannot be deleted while a cluster config or a data storage still presents it. Repoint or delete those first. | | `409 Template has deployments` | A template cannot be deleted while deployments reference it. Delete the deployments first. | --- diff --git a/api/compute/dto/config.go b/api/compute/dto/config.go index 3ea74b69b1..a4ef656be3 100644 --- a/api/compute/dto/config.go +++ b/api/compute/dto/config.go @@ -41,7 +41,7 @@ type SlurmClusterConfigRequest struct { LoginUser string `json:"loginUser"` WorkRoot string `json:"workRoot"` - SSHKeyID string `json:"sshKeyId"` + SSHCredentialID string `json:"sshCredentialId"` } // Validate implements httpx.Validator. @@ -50,7 +50,7 @@ func (r *SlurmClusterConfigRequest) Validate() []httpx.FieldError { c.NotBlank("slurmClusterId", "Slurm cluster id cannot be blank", r.SlurmClusterID) c.NotBlank("loginUser", "Login user cannot be blank", r.LoginUser) c.NotBlank("workRoot", "Work root cannot be blank", r.WorkRoot) - c.NotBlank("sshKeyId", "SSH key id cannot be blank", r.SSHKeyID) + c.NotBlank("sshCredentialId", "SSH credential id cannot be blank", r.SSHCredentialID) return c.Fields() } @@ -81,8 +81,8 @@ type SlurmClusterConfigResponse struct { LoginUser string `json:"loginUser"` WorkRoot string `json:"workRoot"` - SSHKeyID *string `json:"sshKeyId"` - SSHKey *creddto.SSHKeyResponse `json:"sshKey"` + SSHCredentialID *string `json:"sshCredentialId"` + SSHCredential *creddto.SSHCredentialResponse `json:"sshCredential"` Permission *string `json:"permission,omitempty"` } @@ -96,15 +96,15 @@ func ToSlurmClusterConfigResponse(c *model.SlurmClusterConfig) SlurmClusterConfi SlurmClusterID: c.SlurmClusterID, LoginUser: c.LoginUser, WorkRoot: c.WorkRoot, - SSHKeyID: c.SSHKeyID, + SSHCredentialID: c.SSHCredentialID, } if c.SlurmCluster != nil { cluster := ToSlurmClusterResponse(c.SlurmCluster) out.SlurmCluster = &cluster } - if c.SSHKey != nil { - key := creddto.ToSSHKeyResponse(c.SSHKey) - out.SSHKey = &key + if c.SSHCredential != nil { + key := creddto.ToSSHCredentialResponse(c.SSHCredential) + out.SSHCredential = &key } return out } diff --git a/api/compute/model/cluster.go b/api/compute/model/cluster.go index c7fa10b007..a7e2402d64 100644 --- a/api/compute/model/cluster.go +++ b/api/compute/model/cluster.go @@ -92,8 +92,8 @@ type SlurmClusterConfig struct { WorkRoot string `gorm:"column:work_root;type:varchar(1024);not null" json:"workRoot"` - SSHKeyID *string `gorm:"column:ssh_key_id;type:varchar(36);index" json:"sshKeyId,omitempty"` - SSHKey *cred.SSHKey `gorm:"references:ID;constraint:OnDelete:RESTRICT,OnUpdate:CASCADE" json:"sshKey,omitempty"` + SSHCredentialID *string `gorm:"column:ssh_credential_id;type:varchar(36);index" json:"sshCredentialId,omitempty"` + SSHCredential *cred.SSHCredential `gorm:"references:ID;constraint:OnDelete:RESTRICT,OnUpdate:CASCADE" json:"sshCredential,omitempty"` // OwnerID is named for its role because ownership, not mere reference, is what the // authorisation checks read. RESTRICT: a user who still owns configs cannot be diff --git a/api/compute/repository/config.go b/api/compute/repository/config.go index 04b8710240..25ee120906 100644 --- a/api/compute/repository/config.go +++ b/api/compute/repository/config.go @@ -50,7 +50,7 @@ func (r *SlurmClusterConfigRepository) withReferences(ctx context.Context) *gorm return r.db.WithContext(ctx). Preload("SlurmCluster"). Preload("SlurmCluster.Partitions"). - Preload("SSHKey") + Preload("SSHCredential") } // FindAll returns every config. @@ -102,14 +102,14 @@ func (r *SlurmClusterConfigRepository) FindByIDsExcludingOwner(ctx context.Conte return out, err } -// CountByKeyID reports how many configs still present one SSH key. The credentials +// CountByKeyID reports how many configs still present one SSH credential. The credentials // vertical asks before deleting a key: the foreign key is RESTRICT, so without this the // delete would fail as an opaque constraint violation rather than a 409 naming what // still holds it. func (r *SlurmClusterConfigRepository) CountByKeyID(ctx context.Context, keyID string) (int64, error) { var n int64 err := r.db.WithContext(ctx).Model(&model.SlurmClusterConfig{}). - Where("ssh_key_id = ?", keyID).Count(&n).Error + Where("ssh_credential_id = ?", keyID).Count(&n).Error return n, err } diff --git a/api/compute/service/config.go b/api/compute/service/config.go index f8435dac98..bf1f5fb60c 100644 --- a/api/compute/service/config.go +++ b/api/compute/service/config.go @@ -145,7 +145,7 @@ func (a *ConfigAccess) RequireUsable(ctx context.Context, id string) (*model.Slu // SlurmClusterConfigService manages the login configs jobs are submitted through. // // Registering one is self-service: any authenticated caller may declare how they reach -// a cluster from the catalogue, under one of their own SSH keys, and it belongs to +// a cluster from the catalogue, under one of their own SSH credentials, and it belongs to // them. Everyone else reaches it through its sharing rules — which is how they submit // under a key they do not hold. type SlurmClusterConfigService struct { @@ -257,18 +257,18 @@ func (s *SlurmClusterConfigService) Get(ctx context.Context, id string) (*dto.Sl // the caller to own it; keeping the one already there does not, so a grantee with WRITE // can edit a config without owning the key it logs in with — and still cannot point it // at a key of their own. -func (s *SlurmClusterConfigService) resolveReferences(ctx context.Context, tx *gorm.DB, req *dto.SlurmClusterConfigRequest, held *string) (*model.SlurmCluster, *credmodel.SSHKey, error) { +func (s *SlurmClusterConfigService) resolveReferences(ctx context.Context, tx *gorm.DB, req *dto.SlurmClusterConfigRequest, held *string) (*model.SlurmCluster, *credmodel.SSHCredential, error) { cluster, err := s.clusters.WithTx(tx).FindByID(ctx, req.SlurmClusterID) if err != nil { return nil, nil, notFoundAs(err, "Slurm cluster not found: %s", req.SlurmClusterID) } keys := s.keys.WithTx(tx) - if held != nil && *held == req.SSHKeyID { - key, err := keys.Find(ctx, req.SSHKeyID) + if held != nil && *held == req.SSHCredentialID { + key, err := keys.Find(ctx, req.SSHCredentialID) return cluster, key, err } - key, err := keys.RequireOwned(ctx, req.SSHKeyID) + key, err := keys.RequireOwned(ctx, req.SSHCredentialID) if err != nil { return nil, nil, err } @@ -276,7 +276,7 @@ func (s *SlurmClusterConfigService) resolveReferences(ctx context.Context, tx *g } // Create registers a config owned by the calling user, against an existing cluster and -// an existing SSH key. +// an existing SSH credential. // // The owner is taken from the token, so there is no way to register a config on // someone else's behalf. @@ -300,11 +300,11 @@ func (s *SlurmClusterConfigService) Create(ctx context.Context, req *dto.SlurmCl } config := &model.SlurmClusterConfig{ - SlurmClusterID: cluster.ID, - SlurmCluster: cluster, - SSHKeyID: &key.ID, - SSHKey: key, - OwnerID: &owner.ID, + SlurmClusterID: cluster.ID, + SlurmCluster: cluster, + SSHCredentialID: &key.ID, + SSHCredential: key, + OwnerID: &owner.ID, } dto.ApplySlurmClusterConfigRequest(config, req) if err := configs.Save(ctx, config); err != nil { @@ -337,7 +337,7 @@ func (s *SlurmClusterConfigService) Update(ctx context.Context, id string, req * if err != nil { return err } - cluster, key, err := s.resolveReferences(ctx, tx, req, config.SSHKeyID) + cluster, key, err := s.resolveReferences(ctx, tx, req, config.SSHCredentialID) if err != nil { return err } @@ -345,8 +345,8 @@ func (s *SlurmClusterConfigService) Update(ctx context.Context, id string, req * dto.ApplySlurmClusterConfigRequest(config, req) config.SlurmClusterID = cluster.ID config.SlurmCluster = cluster - config.SSHKeyID = &key.ID - config.SSHKey = key + config.SSHCredentialID = &key.ID + config.SSHCredential = key if err := configs.Save(ctx, config); err != nil { return err } diff --git a/api/compute/service/config_test.go b/api/compute/service/config_test.go index 3473b658b5..a23df496f2 100644 --- a/api/compute/service/config_test.go +++ b/api/compute/service/config_test.go @@ -87,7 +87,7 @@ func newConfigFixture(t *testing.T) *configFixture { if err := gdb.Create(cluster).Error; err != nil { t.Fatalf("create cluster: %v", err) } - key := &credmodel.SSHKey{SSHKeyName: "alice-key", PublicKey: "ssh-ed25519 AAAA", PrivateKey: "secret", OwnerID: "alice"} + key := &credmodel.SSHCredential{SSHCredentialName: "alice-key", PublicKey: "ssh-ed25519 AAAA", PrivateKey: "secret", OwnerID: "alice"} if err := gdb.Create(key).Error; err != nil { t.Fatalf("create key: %v", err) } @@ -95,7 +95,7 @@ func newConfigFixture(t *testing.T) *configFixture { configs := computerepo.NewSlurmClusterConfigRepository(gdb) shares := sharingrepo.NewRepository(gdb) clusters := computerepo.NewSlurmClusterRepository(gdb) - keys := credrepo.NewSSHKeyRepository(gdb) + keys := credrepo.NewSSHCredentialRepository(gdb) users := iamrepo.NewUserRepository(gdb) groups := iamrepo.NewGroupRepository(gdb) members := iamrepo.NewGroupMemberRepository(gdb) @@ -114,11 +114,11 @@ func newConfigFixture(t *testing.T) *configFixture { func (f *configFixture) req() *dto.SlurmClusterConfigRequest { return &dto.SlurmClusterConfigRequest{ - Name: str("expanse allocation"), - SlurmClusterID: f.clusterID, - LoginUser: "alice", - WorkRoot: "/scratch/alice", - SSHKeyID: f.keyID, + Name: str("expanse allocation"), + SlurmClusterID: f.clusterID, + LoginUser: "alice", + WorkRoot: "/scratch/alice", + SSHCredentialID: f.keyID, } } @@ -148,8 +148,8 @@ func TestConfigCreateTakesOwnerFromToken(t *testing.T) { if out.SlurmCluster == nil || out.SlurmCluster.ClusterName != "expanse" { t.Errorf("slurmCluster = %+v, want the expanse cluster inlined", out.SlurmCluster) } - if out.SSHKey == nil || out.SSHKey.SSHKeyName != "alice-key" { - t.Errorf("sshKey = %+v, want the key summary inlined", out.SSHKey) + if out.SSHCredential == nil || out.SSHCredential.SSHCredentialName != "alice-key" { + t.Errorf("sshCredential = %+v, want the key summary inlined", out.SSHCredential) } } @@ -350,7 +350,7 @@ func TestConfigReferencesMustResolve(t *testing.T) { } req = f.req() - req.SSHKeyID = "no-such-key" + req.SSHCredentialID = "no-such-key" if _, err := f.configs.Create(f.owner, req); httpx.StatusOf(err) != 404 { t.Errorf("unknown key = %v (status %d), want 404", err, httpx.StatusOf(err)) } diff --git a/api/credentials/controller/controller.go b/api/credentials/controller/controller.go index a46a97c518..2724213044 100644 --- a/api/credentials/controller/controller.go +++ b/api/credentials/controller/controller.go @@ -18,7 +18,7 @@ * under the License. */ -// Package controller serves the SSH key routes. +// Package controller serves the SSH credential routes. package controller import ( @@ -30,24 +30,24 @@ import ( "github.com/apache/airavata/api/credentials/service" ) -// SSHKeyController serves /api/v1/ssh-keys. -type SSHKeyController struct{ svc *service.SSHKeyService } +// SSHCredentialController serves /api/v1/ssh-credentials. +type SSHCredentialController struct{ svc *service.SSHCredentialService } -// NewSSHKeyController returns a handler delegating to svc. -func NewSSHKeyController(svc *service.SSHKeyService) *SSHKeyController { - return &SSHKeyController{svc: svc} +// NewSSHCredentialController returns a handler delegating to svc. +func NewSSHCredentialController(svc *service.SSHCredentialService) *SSHCredentialController { + return &SSHCredentialController{svc: svc} } -// Register mounts the SSH key routes. -func (h *SSHKeyController) Register(mux *http.ServeMux) { - mux.HandleFunc("GET /api/v1/ssh-keys", h.list) - mux.HandleFunc("POST /api/v1/ssh-keys", h.create) - mux.HandleFunc("GET /api/v1/ssh-keys/{sshKeyId}", h.get) - mux.HandleFunc("PUT /api/v1/ssh-keys/{sshKeyId}", h.update) - mux.HandleFunc("DELETE /api/v1/ssh-keys/{sshKeyId}", h.delete) +// Register mounts the SSH credential routes. +func (h *SSHCredentialController) Register(mux *http.ServeMux) { + mux.HandleFunc("GET /api/v1/ssh-credentials", h.list) + mux.HandleFunc("POST /api/v1/ssh-credentials", h.create) + mux.HandleFunc("GET /api/v1/ssh-credentials/{sshCredentialId}", h.get) + mux.HandleFunc("PUT /api/v1/ssh-credentials/{sshCredentialId}", h.update) + mux.HandleFunc("DELETE /api/v1/ssh-credentials/{sshCredentialId}", h.delete) } -func (h *SSHKeyController) list(w http.ResponseWriter, r *http.Request) { +func (h *SSHCredentialController) list(w http.ResponseWriter, r *http.Request) { keys, err := h.svc.List(r.Context()) if err != nil { httpx.WriteError(w, r, err) @@ -56,8 +56,8 @@ func (h *SSHKeyController) list(w http.ResponseWriter, r *http.Request) { httpx.WriteJSON(w, http.StatusOK, keys) } -func (h *SSHKeyController) get(w http.ResponseWriter, r *http.Request) { - key, err := h.svc.Get(r.Context(), r.PathValue("sshKeyId")) +func (h *SSHCredentialController) get(w http.ResponseWriter, r *http.Request) { + key, err := h.svc.Get(r.Context(), r.PathValue("sshCredentialId")) if err != nil { httpx.WriteError(w, r, err) return @@ -65,8 +65,8 @@ func (h *SSHKeyController) get(w http.ResponseWriter, r *http.Request) { httpx.WriteJSON(w, http.StatusOK, key) } -func (h *SSHKeyController) create(w http.ResponseWriter, r *http.Request) { - var req dto.SSHKeyRequest +func (h *SSHCredentialController) create(w http.ResponseWriter, r *http.Request) { + var req dto.SSHCredentialRequest if err := httpx.Bind(r, &req); err != nil { httpx.WriteError(w, r, err) return @@ -79,13 +79,13 @@ func (h *SSHKeyController) create(w http.ResponseWriter, r *http.Request) { httpx.WriteJSON(w, http.StatusCreated, key) } -func (h *SSHKeyController) update(w http.ResponseWriter, r *http.Request) { - var req dto.SSHKeyRequest +func (h *SSHCredentialController) update(w http.ResponseWriter, r *http.Request) { + var req dto.SSHCredentialRequest if err := httpx.Bind(r, &req); err != nil { httpx.WriteError(w, r, err) return } - key, err := h.svc.Update(r.Context(), r.PathValue("sshKeyId"), &req) + key, err := h.svc.Update(r.Context(), r.PathValue("sshCredentialId"), &req) if err != nil { httpx.WriteError(w, r, err) return @@ -93,8 +93,8 @@ func (h *SSHKeyController) update(w http.ResponseWriter, r *http.Request) { httpx.WriteJSON(w, http.StatusOK, key) } -func (h *SSHKeyController) delete(w http.ResponseWriter, r *http.Request) { - if err := h.svc.Delete(r.Context(), r.PathValue("sshKeyId")); err != nil { +func (h *SSHCredentialController) delete(w http.ResponseWriter, r *http.Request) { + if err := h.svc.Delete(r.Context(), r.PathValue("sshCredentialId")); err != nil { httpx.WriteError(w, r, err) return } diff --git a/api/credentials/dto/ssh_key.go b/api/credentials/dto/ssh_credential.go similarity index 65% rename from api/credentials/dto/ssh_key.go rename to api/credentials/dto/ssh_credential.go index 3fd977fc74..5327618d79 100644 --- a/api/credentials/dto/ssh_key.go +++ b/api/credentials/dto/ssh_credential.go @@ -26,29 +26,29 @@ import ( model "github.com/apache/airavata/api/credentials/model" ) -// SSHKeyRequest is the create/update payload for a key. +// SSHCredentialRequest is the create/update payload for a key. // // PrivateKey is optional on the type because the same payload serves both create and // update: creation requires it, while an update that omits it keeps the stored secret. // That split cannot be expressed as a field constraint, so the service enforces it. // // Java: org.apache.airavata.credentials.dto.SSHKeyRequestDto -type SSHKeyRequest struct { - SSHKeyName string `json:"sshKeyName"` - PublicKey string `json:"publicKey"` - PrivateKey *string `json:"privateKey"` - Passphrase *string `json:"passphrase"` +type SSHCredentialRequest struct { + SSHCredentialName string `json:"sshCredentialName"` + PublicKey string `json:"publicKey"` + PrivateKey *string `json:"privateKey"` + Passphrase *string `json:"passphrase"` } // Validate implements httpx.Validator. -func (r *SSHKeyRequest) Validate() []httpx.FieldError { +func (r *SSHCredentialRequest) Validate() []httpx.FieldError { var c httpx.Constraints - c.NotBlank("sshKeyName", "SSH key name cannot be blank", r.SSHKeyName) + c.NotBlank("sshCredentialName", "SSH credential name cannot be blank", r.SSHCredentialName) c.NotBlank("publicKey", "Public key cannot be blank", r.PublicKey) return c.Fields() } -// SSHKeyResponse is the read model for a key. +// SSHCredentialResponse is the read model for a key. // // It has no private key or passphrase field at all. That is the containment: the // secrets cannot leak through this endpoint because there is nowhere for them to go. @@ -58,18 +58,18 @@ func (r *SSHKeyRequest) Validate() []httpx.FieldError { // handed over by editing it. // // Java: org.apache.airavata.credentials.dto.SSHKeyResponseDto -type SSHKeyResponse struct { - SSHKeyID string `json:"sshKeyId"` - SSHKeyName string `json:"sshKeyName"` - PublicKey string `json:"publicKey"` - OwnerID string `json:"ownerId"` +type SSHCredentialResponse struct { + SSHCredentialID string `json:"sshCredentialId"` + SSHCredentialName string `json:"sshCredentialName"` + PublicKey string `json:"publicKey"` + OwnerID string `json:"ownerId"` } -func ToSSHKeyResponse(k *model.SSHKey) SSHKeyResponse { - return SSHKeyResponse{ - SSHKeyID: k.ID, - SSHKeyName: k.SSHKeyName, - PublicKey: k.PublicKey, - OwnerID: k.OwnerID, +func ToSSHCredentialResponse(k *model.SSHCredential) SSHCredentialResponse { + return SSHCredentialResponse{ + SSHCredentialID: k.ID, + SSHCredentialName: k.SSHCredentialName, + PublicKey: k.PublicKey, + OwnerID: k.OwnerID, } } diff --git a/api/credentials/model/ssh_key.go b/api/credentials/model/ssh_credential.go similarity index 57% rename from api/credentials/model/ssh_key.go rename to api/credentials/model/ssh_credential.go index ecd8d73468..7beb19b9b5 100644 --- a/api/credentials/model/ssh_key.go +++ b/api/credentials/model/ssh_credential.go @@ -18,7 +18,7 @@ * under the License. */ -// Package model holds the SSH key entity. +// Package model holds the SSH credential entity. package model import ( @@ -27,7 +27,7 @@ import ( "gorm.io/gorm" ) -// SSHKey is a registered SSH keypair. +// SSHCredential is a registered SSH credential. // // PrivateKey and Passphrase are secrets: they are stored here but deliberately have // no field on the read-side response DTO, so they cannot leak through a GET. Update @@ -35,23 +35,25 @@ import ( // ptr.NonBlank. // // It belongs to whoever registered it, and — unlike every other owned record here — -// there is no sharing model to reach it by. A key is the credential itself rather than -// something reached with one, so a cluster config or a data storage may only present a -// key its own owner registered. Sharing the config is how someone else submits under -// it; the key never leaves its owner. -// -// Java: org.apache.airavata.credentials.model.SSHKeyEntity -type SSHKey struct { - ID string `gorm:"column:ssh_key_id;primaryKey;type:varchar(36)" json:"sshKeyId"` - - SSHKeyName string `gorm:"column:ssh_key_name;type:varchar(255);not null" json:"sshKeyName"` +// there is no sharing model to reach it by. A credential is the secret material itself +// rather than something reached with it, so a cluster config or a data storage may only +// present a credential its own owner registered. Sharing the config is how someone else +// submits under it; the credential never leaves its owner. +type SSHCredential struct { + ID string `gorm:"column:ssh_credential_id;primaryKey;type:varchar(36)" json:"sshCredentialId"` - // @Lob in Java: keys are far longer than a default varchar. - PublicKey string `gorm:"column:public_key;type:text;not null" json:"publicKey"` - PrivateKey string `gorm:"column:private_key;type:text;not null" json:"-"` + SSHCredentialName string `gorm:"column:ssh_credential_name;type:varchar(255);not null" json:"sshCredentialName"` + PublicKey string `gorm:"column:public_key;type:text;not null" json:"publicKey"` + PrivateKey string `gorm:"column:private_key;type:text;not null" json:"-"` Passphrase *string `gorm:"column:passphrase;type:varchar(255)" json:"-"` + // Certificate based authentication. This indicates whether the SSH credential uses a certificate for authentication. + IsCertificate bool `gorm:"column:is_certificate;not null" json:"isCertificate"` + + // FullInteractive indicates whether the SSH requires full interactive authentication. + FullInteractive bool `gorm:"column:full_interactive;not null" json:"fullInteractive"` + // OwnerID is varchar(255) to match users.user_id: a CILogon subject is far longer // than a UUID. RESTRICT, so a user who still owns keys cannot be deleted out from // under them. @@ -59,18 +61,16 @@ type SSHKey struct { Owner *iam.User `gorm:"references:ID;constraint:OnDelete:RESTRICT,OnUpdate:CASCADE" json:"-"` } -// OwnedBy reports whether userID owns this key. A key with no owner is owned by -// nobody, so it must not match the empty principal name. -func (k *SSHKey) OwnedBy(userID string) bool { +func (k *SSHCredential) OwnedBy(userID string) bool { return k.OwnerID != "" && k.OwnerID == userID } -// TableName returns the table backing SSHKey. -func (SSHKey) TableName() string { return "ssh_keys" } +// TableName returns the table backing SSHCredential. +func (SSHCredential) TableName() string { return "ssh_credentials" } // BeforeCreate assigns a UUID when none was supplied, replacing Hibernate's // @GeneratedValue(strategy = GenerationType.UUID). -func (k *SSHKey) BeforeCreate(*gorm.DB) error { +func (k *SSHCredential) BeforeCreate(*gorm.DB) error { if k.ID == "" { k.ID = uuid.NewString() } diff --git a/api/credentials/repository/repository.go b/api/credentials/repository/repository.go index 820ae8fdf0..03402c5465 100644 --- a/api/credentials/repository/repository.go +++ b/api/credentials/repository/repository.go @@ -18,7 +18,7 @@ * under the License. */ -// Package repository reads and writes registered SSH keys. +// Package repository reads and writes registered SSH credentials. package repository import ( @@ -29,53 +29,57 @@ import ( model "github.com/apache/airavata/api/credentials/model" ) -// SSHKeyRepository reads and writes registered SSH keypairs. -type SSHKeyRepository struct{ db *gorm.DB } +// SSHCredentialRepository reads and writes registered SSH credentials. +type SSHCredentialRepository struct{ db *gorm.DB } -// NewSSHKeyRepository returns a repository backed by db. -func NewSSHKeyRepository(db *gorm.DB) *SSHKeyRepository { return &SSHKeyRepository{db: db} } +// NewSSHCredentialRepository returns a repository backed by db. +func NewSSHCredentialRepository(db *gorm.DB) *SSHCredentialRepository { + return &SSHCredentialRepository{db: db} +} // WithTx returns a repository bound to tx. -func (r *SSHKeyRepository) WithTx(tx *gorm.DB) *SSHKeyRepository { return &SSHKeyRepository{db: tx} } +func (r *SSHCredentialRepository) WithTx(tx *gorm.DB) *SSHCredentialRepository { + return &SSHCredentialRepository{db: tx} +} // FindAll returns every key. -func (r *SSHKeyRepository) FindAll(ctx context.Context) ([]model.SSHKey, error) { - var out []model.SSHKey +func (r *SSHCredentialRepository) FindAll(ctx context.Context) ([]model.SSHCredential, error) { + var out []model.SSHCredential err := r.db.WithContext(ctx).Find(&out).Error return out, err } // FindByOwnerID returns every key registered by one user. There is no listing across -// owners: a key is private to whoever registered it. -func (r *SSHKeyRepository) FindByOwnerID(ctx context.Context, userID string) ([]model.SSHKey, error) { - var out []model.SSHKey +// owners: a credential is private to whoever registered it. +func (r *SSHCredentialRepository) FindByOwnerID(ctx context.Context, userID string) ([]model.SSHCredential, error) { + var out []model.SSHCredential err := r.db.WithContext(ctx).Where("owner_id = ?", userID).Find(&out).Error return out, err } // FindByID returns one key, or gorm.ErrRecordNotFound. -func (r *SSHKeyRepository) FindByID(ctx context.Context, id string) (*model.SSHKey, error) { - var out model.SSHKey - if err := r.db.WithContext(ctx).First(&out, "ssh_key_id = ?", id).Error; err != nil { +func (r *SSHCredentialRepository) FindByID(ctx context.Context, id string) (*model.SSHCredential, error) { + var out model.SSHCredential + if err := r.db.WithContext(ctx).First(&out, "ssh_credential_id = ?", id).Error; err != nil { return nil, err } return &out, nil } // Save inserts or updates a key. -func (r *SSHKeyRepository) Save(ctx context.Context, k *model.SSHKey) error { +func (r *SSHCredentialRepository) Save(ctx context.Context, k *model.SSHCredential) error { return r.db.WithContext(ctx).Save(k).Error } // Delete removes a key. -func (r *SSHKeyRepository) Delete(ctx context.Context, k *model.SSHKey) error { +func (r *SSHCredentialRepository) Delete(ctx context.Context, k *model.SSHCredential) error { return r.db.WithContext(ctx).Delete(k).Error } // ExistsByName reports whether a key of that name is already registered. Declared but // never called in the Java service, and preserved here for the same reason. -func (r *SSHKeyRepository) ExistsByName(ctx context.Context, name string) (bool, error) { +func (r *SSHCredentialRepository) ExistsByName(ctx context.Context, name string) (bool, error) { var n int64 - err := r.db.WithContext(ctx).Model(&model.SSHKey{}).Where("ssh_key_name = ?", name).Count(&n).Error + err := r.db.WithContext(ctx).Model(&model.SSHCredential{}).Where("ssh_credential_name = ?", name).Count(&n).Error return n > 0, err } diff --git a/api/credentials/service/service.go b/api/credentials/service/service.go index 211f5665b8..2d577f5248 100644 --- a/api/credentials/service/service.go +++ b/api/credentials/service/service.go @@ -57,10 +57,12 @@ type KeyReferrer interface { // A cluster config and a data storage both name a key, and both ask this before // pointing at one. Exposing the question rather than the table is what keeps a single // definition of who may present a key — its owner, nobody else. -type KeyAccess struct{ keys *repository.SSHKeyRepository } +type KeyAccess struct { + keys *repository.SSHCredentialRepository +} // NewKeyAccess returns a checker over the key table. -func NewKeyAccess(keys *repository.SSHKeyRepository) *KeyAccess { +func NewKeyAccess(keys *repository.SSHCredentialRepository) *KeyAccess { return &KeyAccess{keys: keys} } @@ -72,10 +74,10 @@ func (a *KeyAccess) WithTx(tx *gorm.DB) *KeyAccess { // Find loads a key without asking who owns it. It is for a record re-presenting the // key it already holds: keeping a key is not assigning one, so someone editing a // config shared with them does not need to own the key already on it. -func (a *KeyAccess) Find(ctx context.Context, id string) (*model.SSHKey, error) { +func (a *KeyAccess) Find(ctx context.Context, id string) (*model.SSHCredential, error) { key, err := a.keys.FindByID(ctx, id) if err != nil { - return nil, notFoundAs(err, "SSH key not found: %s", id) + return nil, notFoundAs(err, "SSH credential not found: %s", id) } return key, nil } @@ -84,7 +86,7 @@ func (a *KeyAccess) Find(ctx context.Context, id string) (*model.SSHKey, error) // when it belongs to somebody else. Assigning a key is presenting its private material // under a name of your choosing, which is the owner's to allow — and they allow it by // sharing what holds the key, never the key. -func (a *KeyAccess) RequireOwned(ctx context.Context, id string) (*model.SSHKey, error) { +func (a *KeyAccess) RequireOwned(ctx context.Context, id string) (*model.SSHCredential, error) { principal, err := auth.RequireAuthenticated(ctx) if err != nil { return nil, err @@ -94,35 +96,36 @@ func (a *KeyAccess) RequireOwned(ctx context.Context, id string) (*model.SSHKey, return nil, err } if !key.OwnedBy(principal.Name) { - return nil, httpx.Forbidden("Access denied: SSH key %s belongs to another user", id) + return nil, httpx.Forbidden("Access denied: SSH credential %s belongs to another user", id) } return key, nil } -// SSHKeyService manages registered SSH keypairs. +// SSHCredentialService manages registered SSH credentials. // -// Registering one is self-service — any authenticated caller may register a key, and it +// Registering one is self-service — any authenticated caller may register one, and it // belongs to them — but nothing about it is reachable by anyone else. Reads, updates -// and deletes are the owner's alone, and there is no share to open one up: a key is the -// credential itself, so lending it out is what sharing a cluster config is for. +// and deletes are the owner's alone, and there is no share to open one up: a credential +// is the secret material itself, so lending it out is what sharing a cluster config is +// for. // // Platform admins are deliberately not treated as owners here, unlike everywhere else // in this API. An admin has no business reading or repointing someone's key material. -type SSHKeyService struct { - keys *repository.SSHKeyRepository +type SSHCredentialService struct { + keys *repository.SSHCredentialRepository users *iamrepo.UserRepository usedBy []KeyReferrer } -// NewSSHKeyService returns an SSH key service. usedBy is every kind of record that can -// hold a key; a key none of them names is free to delete. -func NewSSHKeyService(keys *repository.SSHKeyRepository, users *iamrepo.UserRepository, usedBy ...KeyReferrer) *SSHKeyService { - return &SSHKeyService{keys: keys, users: users, usedBy: usedBy} +// NewSSHCredentialService returns an SSH credential service. usedBy is every kind of +// record that can hold a credential; one that none of them names is free to delete. +func NewSSHCredentialService(keys *repository.SSHCredentialRepository, users *iamrepo.UserRepository, usedBy ...KeyReferrer) *SSHCredentialService { + return &SSHCredentialService{keys: keys, users: users, usedBy: usedBy} } -// List returns the caller's own keys. There is no listing across owners: a key is -// private to whoever registered it, and that holds for admins too. -func (s *SSHKeyService) List(ctx context.Context) ([]dto.SSHKeyResponse, error) { +// List returns the caller's own credentials. There is no listing across owners: a +// credential is private to whoever registered it, and that holds for admins too. +func (s *SSHCredentialService) List(ctx context.Context) ([]dto.SSHCredentialResponse, error) { principal, err := auth.RequireAuthenticated(ctx) if err != nil { return nil, err @@ -131,20 +134,20 @@ func (s *SSHKeyService) List(ctx context.Context) ([]dto.SSHKeyResponse, error) if err != nil { return nil, err } - out := make([]dto.SSHKeyResponse, 0, len(keys)) + out := make([]dto.SSHCredentialResponse, 0, len(keys)) for i := range keys { - out = append(out, dto.ToSSHKeyResponse(&keys[i])) + out = append(out, dto.ToSSHCredentialResponse(&keys[i])) } return out, nil } // Get returns one key. -func (s *SSHKeyService) Get(ctx context.Context, id string) (*dto.SSHKeyResponse, error) { +func (s *SSHCredentialService) Get(ctx context.Context, id string) (*dto.SSHCredentialResponse, error) { key, err := s.requireOwnedKey(ctx, id) if err != nil { return nil, err } - out := dto.ToSSHKeyResponse(key) + out := dto.ToSSHCredentialResponse(key) return &out, nil } @@ -154,7 +157,7 @@ func (s *SSHKeyService) Get(ctx context.Context, id string) (*dto.SSHKeyResponse // else's behalf. The private key is required here even though the payload makes it // optional: the same payload is reused for updates, where omitting it means "keep what // is stored". -func (s *SSHKeyService) Create(ctx context.Context, req *dto.SSHKeyRequest) (*dto.SSHKeyResponse, error) { +func (s *SSHCredentialService) Create(ctx context.Context, req *dto.SSHCredentialRequest) (*dto.SSHCredentialResponse, error) { owner, err := iamsvc.RequireCurrentUser(ctx, s.users) if err != nil { return nil, err @@ -163,17 +166,17 @@ func (s *SSHKeyService) Create(ctx context.Context, req *dto.SSHKeyRequest) (*dt return nil, httpx.BadRequest("Private key is required when creating a key") } - key := &model.SSHKey{ - SSHKeyName: req.SSHKeyName, - PublicKey: req.PublicKey, - PrivateKey: *req.PrivateKey, - Passphrase: ptr.NonBlank(req.Passphrase), - OwnerID: owner.ID, + key := &model.SSHCredential{ + SSHCredentialName: req.SSHCredentialName, + PublicKey: req.PublicKey, + PrivateKey: *req.PrivateKey, + Passphrase: ptr.NonBlank(req.Passphrase), + OwnerID: owner.ID, } if err := s.keys.Save(ctx, key); err != nil { return nil, err } - out := dto.ToSSHKeyResponse(key) + out := dto.ToSSHCredentialResponse(key) return &out, nil } @@ -182,13 +185,13 @@ func (s *SSHKeyService) Create(ctx context.Context, req *dto.SSHKeyRequest) (*dt // A blank or absent private key or passphrase means "leave the stored secret alone", // not "erase it". Without that rule, a client round-tripping a response — which never // contains the secrets — would silently wipe them. -func (s *SSHKeyService) Update(ctx context.Context, id string, req *dto.SSHKeyRequest) (*dto.SSHKeyResponse, error) { +func (s *SSHCredentialService) Update(ctx context.Context, id string, req *dto.SSHCredentialRequest) (*dto.SSHCredentialResponse, error) { key, err := s.requireOwnedKey(ctx, id) if err != nil { return nil, err } - key.SSHKeyName = req.SSHKeyName + key.SSHCredentialName = req.SSHCredentialName key.PublicKey = req.PublicKey if v := ptr.NonBlank(req.PrivateKey); v != nil { key.PrivateKey = *v @@ -200,7 +203,7 @@ func (s *SSHKeyService) Update(ctx context.Context, id string, req *dto.SSHKeyRe if err := s.keys.Save(ctx, key); err != nil { return nil, err } - out := dto.ToSSHKeyResponse(key) + out := dto.ToSSHCredentialResponse(key) return &out, nil } @@ -211,7 +214,7 @@ func (s *SSHKeyService) Update(ctx context.Context, id string, req *dto.SSHKeyRe // many records still hold the key. The count runs before the delete rather than in a // transaction with it — as the data storage service checks its products — and the // constraint remains the backstop for anything registered in between. -func (s *SSHKeyService) Delete(ctx context.Context, id string) error { +func (s *SSHCredentialService) Delete(ctx context.Context, id string) error { key, err := s.requireOwnedKey(ctx, id) if err != nil { return err @@ -226,22 +229,22 @@ func (s *SSHKeyService) Delete(ctx context.Context, id string) error { held += n } if held > 0 { - return httpx.Conflict("SSH key %s is still presented by %d record(s) and cannot be deleted", id, held) + return httpx.Conflict("SSH credential %s is still presented by %d record(s) and cannot be deleted", id, held) } return s.keys.Delete(ctx, key) } -func (s *SSHKeyService) requireOwnedKey(ctx context.Context, id string) (*model.SSHKey, error) { +func (s *SSHCredentialService) requireOwnedKey(ctx context.Context, id string) (*model.SSHCredential, error) { principal, err := auth.RequireAuthenticated(ctx) if err != nil { return nil, err } key, err := s.keys.FindByID(ctx, id) if err != nil { - return nil, notFoundAs(err, "SSH key not found: %s", id) + return nil, notFoundAs(err, "SSH credential not found: %s", id) } if !key.OwnedBy(principal.Name) { - return nil, httpx.Forbidden("Access denied: SSH key %s belongs to another user", id) + return nil, httpx.Forbidden("Access denied: SSH credential %s belongs to another user", id) } return key, nil } diff --git a/api/data/dto/storage.go b/api/data/dto/storage.go index dd8c9807fa..2c56cbb9dc 100644 --- a/api/data/dto/storage.go +++ b/api/data/dto/storage.go @@ -45,7 +45,7 @@ type SCPDataStorageRequest struct { LoginUser string `json:"loginUser"` - SSHKeyID string `json:"sshKeyId"` + SSHCredentialID string `json:"sshCredentialId"` } // Validate implements httpx.Validator. @@ -54,7 +54,7 @@ func (r *SCPDataStorageRequest) Validate() []httpx.FieldError { c.NotBlankPtr("dataName", "Data name cannot be blank", r.DataName) c.NotBlank("hostName", "Host name cannot be blank", r.HostName) c.NotBlank("loginUser", "Login user cannot be blank", r.LoginUser) - c.NotBlank("sshKeyId", "SSH key id cannot be blank", r.SSHKeyID) + c.NotBlank("sshCredentialId", "SSH credential id cannot be blank", r.SSHCredentialID) if r.Port != nil && (*r.Port < 1 || *r.Port > 65535) { c.Add("port", "Port must be between 1 and 65535") } @@ -89,25 +89,25 @@ type SCPDataStorageResponse struct { LoginUser *string `json:"loginUser"` - SSHKeyID *string `json:"sshKeyId"` - SSHKey *creddto.SSHKeyResponse `json:"sshKey"` + SSHCredentialID *string `json:"sshCredentialId"` + SSHCredential *creddto.SSHCredentialResponse `json:"sshCredential"` Permission *string `json:"permission,omitempty"` } func ToSCPDataStorageResponse(s *model.SCPDataStorage) SCPDataStorageResponse { out := SCPDataStorageResponse{ - DataID: s.ID, - DataName: s.Name, - OwnerID: s.OwnerID, - HostName: s.HostName, - Port: s.Port, - LoginUser: s.LoginUser, - SSHKeyID: s.SSHKeyID, + DataID: s.ID, + DataName: s.Name, + OwnerID: s.OwnerID, + HostName: s.HostName, + Port: s.Port, + LoginUser: s.LoginUser, + SSHCredentialID: s.SSHCredentialID, } - if s.SSHKey != nil { - key := creddto.ToSSHKeyResponse(s.SSHKey) - out.SSHKey = &key + if s.SSHCredential != nil { + key := creddto.ToSSHCredentialResponse(s.SSHCredential) + out.SSHCredential = &key } return out } diff --git a/api/data/model/storage.go b/api/data/model/storage.go index 92238d86d8..606ab0c4a3 100644 --- a/api/data/model/storage.go +++ b/api/data/model/storage.go @@ -67,8 +67,8 @@ type SCPDataStorage struct { LoginUser *string `gorm:"column:login_user;type:varchar(255)" json:"loginUser,omitempty"` - SSHKeyID *string `gorm:"column:ssh_key_id;type:varchar(36);index" json:"sshKeyId,omitempty"` - SSHKey *cred.SSHKey `gorm:"references:ID;constraint:OnDelete:RESTRICT,OnUpdate:CASCADE" json:"sshKey,omitempty"` + SSHCredentialID *string `gorm:"column:ssh_credential_id;type:varchar(36);index" json:"sshCredentialId,omitempty"` + SSHCredential *cred.SSHCredential `gorm:"references:ID;constraint:OnDelete:RESTRICT,OnUpdate:CASCADE" json:"sshCredential,omitempty"` // OwnerID is named for its role because ownership, not mere reference, is what the // authorisation checks read. RESTRICT: a user who still owns storages cannot be diff --git a/api/data/repository/storage.go b/api/data/repository/storage.go index 0260a7260d..a920ce253c 100644 --- a/api/data/repository/storage.go +++ b/api/data/repository/storage.go @@ -30,7 +30,7 @@ import ( // SCPDataStorageRepository reads and writes SCP data storages. // -// Reads preload the SSH key because the response DTO carries its public summary: the +// Reads preload the SSH credential because the response DTO carries its public summary: the // host and the login user are columns on the storage itself, but the key it presents // is a reference, and a storage without it is not much use to a caller deciding // whether to put data there. @@ -48,7 +48,7 @@ func (r *SCPDataStorageRepository) WithTx(tx *gorm.DB) *SCPDataStorageRepository // withReferences is the read scope every lookup that feeds a response DTO starts from. func (r *SCPDataStorageRepository) withReferences(ctx context.Context) *gorm.DB { - return r.db.WithContext(ctx).Preload("SSHKey") + return r.db.WithContext(ctx).Preload("SSHCredential") } // FindAll returns every storage. @@ -91,13 +91,13 @@ func (r *SCPDataStorageRepository) FindByIDsExcludingOwner(ctx context.Context, return out, err } -// CountByKeyID reports how many storages still present one SSH key, for the same +// CountByKeyID reports how many storages still present one SSH credential, for the same // reason the cluster configs do: the key's foreign key is RESTRICT, and a 409 naming // what holds it beats a constraint violation. func (r *SCPDataStorageRepository) CountByKeyID(ctx context.Context, keyID string) (int64, error) { var n int64 err := r.db.WithContext(ctx).Model(&model.SCPDataStorage{}). - Where("ssh_key_id = ?", keyID).Count(&n).Error + Where("ssh_credential_id = ?", keyID).Count(&n).Error return n, err } diff --git a/api/data/service/storage.go b/api/data/service/storage.go index 2329171bb8..1f548597c2 100644 --- a/api/data/service/storage.go +++ b/api/data/service/storage.go @@ -116,7 +116,7 @@ func requireStorageReadable(ctx context.Context, base access, sharing *sharingre // // Registering one is self-service: any authenticated caller may declare a storage on a // host of their choosing, under an account of their choosing, presenting one of their -// own SSH keys, and it belongs to them. Everyone else reaches it through its sharing +// own SSH credentials, and it belongs to them. Everyone else reaches it through its sharing // rules — which is how they stage under a key they do not hold. type SCPDataStorageService struct { storageAccess @@ -219,7 +219,7 @@ func (s *SCPDataStorageService) Get(ctx context.Context, id string) (*dto.SCPDat return &out, nil } -// resolveKey loads the SSH key a request names. It is not created here, so an id that +// resolveKey loads the SSH credential a request names. It is not created here, so an id that // resolves to nothing is a 404 rather than a storage pointing at a key that does not // exist. // @@ -227,16 +227,16 @@ func (s *SCPDataStorageService) Get(ctx context.Context, id string) (*dto.SCPDat // the caller to own it; keeping the one already there does not, so a grantee with WRITE // can rename a storage or move its host without owning the key it stages under — and // still cannot point it at a key of their own. -func (s *SCPDataStorageService) resolveKey(ctx context.Context, tx *gorm.DB, req *dto.SCPDataStorageRequest, held *string) (*credmodel.SSHKey, error) { +func (s *SCPDataStorageService) resolveKey(ctx context.Context, tx *gorm.DB, req *dto.SCPDataStorageRequest, held *string) (*credmodel.SSHCredential, error) { keys := s.keys.WithTx(tx) - if held != nil && *held == req.SSHKeyID { - return keys.Find(ctx, req.SSHKeyID) + if held != nil && *held == req.SSHCredentialID { + return keys.Find(ctx, req.SSHCredentialID) } - return keys.RequireOwned(ctx, req.SSHKeyID) + return keys.RequireOwned(ctx, req.SSHCredentialID) } // Create registers a storage owned by the calling user, on the host it names and under -// an existing SSH key. +// an existing SSH credential. // // The owner is taken from the token, so there is no way to register a storage on // someone else's behalf. @@ -260,9 +260,9 @@ func (s *SCPDataStorageService) Create(ctx context.Context, req *dto.SCPDataStor } storage := &model.SCPDataStorage{ - SSHKeyID: &key.ID, - SSHKey: key, - OwnerID: &owner.ID, + SSHCredentialID: &key.ID, + SSHCredential: key, + OwnerID: &owner.ID, } dto.ApplySCPDataStorageRequest(storage, req) if err := storages.Save(ctx, storage); err != nil { @@ -295,14 +295,14 @@ func (s *SCPDataStorageService) Update(ctx context.Context, id string, req *dto. if err != nil { return err } - key, err := s.resolveKey(ctx, tx, req, storage.SSHKeyID) + key, err := s.resolveKey(ctx, tx, req, storage.SSHCredentialID) if err != nil { return err } dto.ApplySCPDataStorageRequest(storage, req) - storage.SSHKeyID = &key.ID - storage.SSHKey = key + storage.SSHCredentialID = &key.ID + storage.SSHCredential = key if err := storages.Save(ctx, storage); err != nil { return err } diff --git a/api/data/service/transfer.go b/api/data/service/transfer.go index b0ff96e531..918064b80f 100644 --- a/api/data/service/transfer.go +++ b/api/data/service/transfer.go @@ -49,7 +49,7 @@ type Endpoint struct { Host string Port int LoginUser string - Key credmodel.SSHKey + Key credmodel.SSHCredential } // Transport moves a single file between this server and the host an Endpoint names. @@ -252,7 +252,7 @@ func (s *DataTransferService) locate(ctx context.Context, product *model.DataPro if err != nil { return "", ep, notFoundAs(err, "SCP data storage not found: %s", *product.DataStorageID) } - if storage.HostName == nil || storage.LoginUser == nil || storage.SSHKey == nil { + if storage.HostName == nil || storage.LoginUser == nil || storage.SSHCredential == nil { return "", ep, httpx.Conflict( "SCP data storage %s is missing the host, login user or key a transfer needs", storage.ID) } @@ -265,7 +265,7 @@ func (s *DataTransferService) locate(ctx context.Context, product *model.DataPro Host: *storage.HostName, Port: port, LoginUser: *storage.LoginUser, - Key: *storage.SSHKey, + Key: *storage.SSHCredential, }, nil } diff --git a/api/data/service/transport.go b/api/data/service/transport.go index 8eeaa0b896..4ebc1e1b1e 100644 --- a/api/data/service/transport.go +++ b/api/data/service/transport.go @@ -23,7 +23,7 @@ package service import ( "context" - "github.com/apache/airavata/internal/orchestration" + ssh "github.com/apache/airavata/internal/ssh" ) type SCPTransport struct{} @@ -32,13 +32,13 @@ type SCPTransport struct{} func NewSCPTransport() SCPTransport { return SCPTransport{} } func (SCPTransport) Download(ctx context.Context, ep Endpoint, remotePath, localPath string) error { - return orchestration.DownloadFileFromSCP(ctx, ep.Host, ep.Port, ep.LoginUser, ep.Key, remotePath, localPath) + return ssh.DownloadFileFromSCP(ctx, ep.Host, ep.Port, ep.LoginUser, ep.Key, remotePath, localPath) } func (SCPTransport) Upload(ctx context.Context, ep Endpoint, localPath, remotePath string) error { - return orchestration.UploadFileToSCP(ctx, ep.Host, ep.Port, ep.LoginUser, ep.Key, localPath, remotePath) + return ssh.UploadFileToSCP(ctx, ep.Host, ep.Port, ep.LoginUser, ep.Key, localPath, remotePath) } func (SCPTransport) MakeDirectory(ctx context.Context, ep Endpoint, remoteDir string) error { - return orchestration.CreateDirectorySSH(ctx, ep.Host, ep.Port, ep.LoginUser, ep.Key, remoteDir) + return ssh.CreateDirectorySSH(ctx, ep.Host, ep.Port, ep.LoginUser, ep.Key, remoteDir) } diff --git a/api/process/service/launch.go b/api/process/service/launch.go index 5991dbc30b..8d3dfaf065 100644 --- a/api/process/service/launch.go +++ b/api/process/service/launch.go @@ -54,7 +54,7 @@ type LaunchService struct { templateService *appserv.TemplateService clusterService *computesev.SlurmClusterService clusterConfigService *computesev.SlurmClusterConfigService - sshKeyService *credentialsev.SSHKeyService + sshCredentialService *credentialsev.SSHCredentialService scpDataStorageService *datasev.SCPDataStorageService dataProductService *datasev.DataProductService statusService *StatusService @@ -75,7 +75,7 @@ func NewLaunchService( templateService *appserv.TemplateService, clusterService *computesev.SlurmClusterService, clusterConfigService *computesev.SlurmClusterConfigService, - sshKeyService *credentialsev.SSHKeyService, + sshCredentialService *credentialsev.SSHCredentialService, scpDataStorageService *datasev.SCPDataStorageService, dataProductService *datasev.DataProductService, statusService *StatusService, @@ -94,7 +94,7 @@ func NewLaunchService( templateService: templateService, clusterService: clusterService, clusterConfigService: clusterConfigService, - sshKeyService: sshKeyService, + sshCredentialService: sshCredentialService, scpDataStorageService: scpDataStorageService, dataProductService: dataProductService, statusService: statusService, diff --git a/docs/api.md b/docs/api.md index 253317f6bd..3e091fd279 100644 --- a/docs/api.md +++ b/docs/api.md @@ -4,7 +4,7 @@ Base URL: `http://localhost:9095` (default `SERVER_PORT` is `9095`; override via This page is the narrative reference, with a worked `curl` example per endpoint. [`openapi.yaml`](openapi.yaml) beside it is the same API as a machine-readable OpenAPI 3.1 description, for client generation and for loading into Swagger UI or Redoc.\n\nSharing works the same way for every record that supports it, so it is documented once in **[sharing.md](sharing.md)** — the access model, who may grant what, and worked examples — rather than repeated under each resource below. -All request/response bodies are JSON (`Content-Type: application/json`). Writes require an `Authorization: Bearer ` header for a principal with `ADMIN` or `SUPER_ADMIN` authority; catalog reads (`GET`) are open without a token. [SSH keys](#ssh-keys), [groups](#groups), [cluster configs](#slurm-cluster-configs), [data products](#data-products) and [SCP data storages](#scp-data-storages) are the exception on both counts — they are reached through ownership (and, for all but a key, sharing rules) rather than platform roles, so any authenticated caller may create them, and none are readable anonymously. See INSTALL.md for how to obtain the root token. +All request/response bodies are JSON (`Content-Type: application/json`). Writes require an `Authorization: Bearer ` header for a principal with `ADMIN` or `SUPER_ADMIN` authority; catalog reads (`GET`) are open without a token. [SSH credentials](#ssh-credentials), [groups](#groups), [cluster configs](#slurm-cluster-configs), [data products](#data-products) and [SCP data storages](#scp-data-storages) are the exception on both counts — they are reached through ownership (and, for all but a credential, sharing rules) rather than platform roles, so any authenticated caller may create them, and none are readable anonymously. See INSTALL.md for how to obtain the root token. ## Error responses @@ -41,66 +41,66 @@ The statuses in use: | `401 Unauthorized` | no token where one is required, or a token that is present but unusable — the caller can fix this by presenting credentials. An invalid token also carries `WWW-Authenticate: Bearer error="invalid_token"` | | `403 Forbidden` | authenticated, but lacking the authority for this call | | `404 Not Found` | no such record — or, for [groups](#groups), a record the caller has no standing to know exists | -| `409 Conflict` | the request collides with existing state, e.g. deleting an SSH key still in use | +| `409 Conflict` | the request collides with existing state, e.g. deleting an SSH credential still in use | | `413 Content Too Large` | the body ran past a size cap — only [data product content](#upload-and-download-data-product-content) has one | | `502 Bad Gateway` | an upstream could not be reached: the identity provider when validating a token, or the storage host when [transferring data product content](#upload-and-download-data-product-content). Not the caller's fault either way | Internal failures return `500` with a fixed `"Internal server error"` message: the underlying detail is logged rather than returned, since it may name internal state. -## SSH Keys +## SSH Credentials -An SSH key is a registered keypair, and it belongs to whoever registered it. Registering one is self-service — any authenticated caller may — but nothing about it is reachable by anyone else: reading, updating and deleting a key are the owner's alone, and the listing returns only the caller's own keys. +An SSH credential is a registered keypair, and it belongs to whoever registered it. Registering one is self-service — any authenticated caller may — but nothing about it is reachable by anyone else: reading, updating and deleting a credential are the owner's alone, and the listing returns only the caller's own. -**Platform admins are not treated as owners here**, unlike everywhere else in this API. There is no share to open a key up either. A key is the credential itself rather than something reached with one, so lending it out is what sharing a [cluster config](#share-a-slurm-cluster-config) or a [data storage](#share-an-scp-data-storage) is for — the key stays with its owner while other people submit and stage under it. +**Platform admins are not treated as owners here**, unlike everywhere else in this API. There is no share to open a credential up either. A credential *is* the secret material rather than something reached with it, so lending it out is what sharing a [cluster config](#share-a-slurm-cluster-config) or a [data storage](#share-an-scp-data-storage) is for — the credential stays with its owner while other people submit and stage under it. -A key is what a [Slurm cluster config](#slurm-cluster-configs) presents when it logs in to a cluster, and what an [SCP data storage](#scp-data-storages) presents when data is staged through it. Each of those carries its own host and login user, so the key is the only thing they take from here — and the only reason a key cannot be deleted. **Putting a key on either one requires owning it**: assigning a key means presenting its private half under a name of your choosing, so a request naming somebody else's key is `403 Forbidden`. Re-presenting the key a record already holds is not an assignment, which is what lets a `WRITE` grantee edit a config or a storage without owning the key on it. +A credential is what a [Slurm cluster config](#slurm-cluster-configs) presents when it logs in to a cluster, and what an [SCP data storage](#scp-data-storages) presents when data is staged through it. Each of those carries its own host and login user, so the credential is the only thing they take from here — and the only reason a credential cannot be deleted. **Putting a credential on either one requires owning it**: assigning one means presenting its private half under a name of your choosing, so a request naming somebody else's credential is `403 Forbidden`. Re-presenting the credential a record already holds is not an assignment, which is what lets a `WRITE` grantee edit a config or a storage without owning the credential on it. ``` -GET /api/v1/ssh-keys # the caller's own keys -POST /api/v1/ssh-keys -GET /api/v1/ssh-keys/{sshKeyId} (owner) -PUT /api/v1/ssh-keys/{sshKeyId} (owner) -DELETE /api/v1/ssh-keys/{sshKeyId} (owner) +GET /api/v1/ssh-credentials # the caller's own credentials +POST /api/v1/ssh-credentials +GET /api/v1/ssh-credentials/{sshCredentialId} (owner) +PUT /api/v1/ssh-credentials/{sshCredentialId} (owner) +DELETE /api/v1/ssh-credentials/{sshCredentialId} (owner) ``` -### Create SSH Key +### Create SSH Credential ``` -POST /api/v1/ssh-keys +POST /api/v1/ssh-credentials ``` Requires an authenticated principal with a `users` row (see INSTALL.md's "Owning resources requires a matching `users` row"). The owner is taken from the token — there is no owner field in the body — and is not transferable afterwards. Stores the keypair used to authenticate to a host — a cluster head node through a [cluster config](#slurm-cluster-configs), or a data host through an [SCP data storage](#scp-data-storages); `privateKey` and `passphrase` are write-only — they are never returned by any read endpoint. **curl example** -The example below captures `sshKeyId` from the response into `$SSH_KEY_ID` (requires `jq`), for use by the cluster config and the data storage below. +The example below captures `sshCredentialId` from the response into `$SSH_CREDENTIAL_ID` (requires `jq`), for use by the cluster config and the data storage below. ```bash -SSH_KEY_ID=$(curl -s -X POST localhost:9095/api/v1/ssh-keys \ +SSH_CREDENTIAL_ID=$(curl -s -X POST localhost:9095/api/v1/ssh-credentials \ -H "Authorization: Bearer $TOKEN" \ -H "Content-Type: application/json" \ -d '{ - "sshKeyName": "expanse-key", + "sshCredentialName": "expanse-key", "publicKey": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIExampleKeyDoNotUse airavata@expanse", "privateKey": "-----BEGIN OPENSSH PRIVATE KEY-----\nExampleKeyDoNotUse\n-----END OPENSSH PRIVATE KEY-----", "passphrase": null - }' | jq -r '.sshKeyId') + }' | jq -r '.sshCredentialId') -echo "$SSH_KEY_ID" +echo "$SSH_CREDENTIAL_ID" ``` **Request body** | Field | Type | Notes | |---|---|---| -| `sshKeyName` | string | required, cannot be blank | +| `sshCredentialName` | string | required, cannot be blank | | `publicKey` | string | required, cannot be blank | | `privateKey` | string \| null | required on create; omit on update to keep the stored secret | | `passphrase` | string \| null | optional | ```json { - "sshKeyName": "expanse-key", + "sshCredentialName": "expanse-key", "publicKey": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIExampleKeyDoNotUse airavata@expanse", "privateKey": "-----BEGIN OPENSSH PRIVATE KEY-----\nExampleKeyDoNotUse\n-----END OPENSSH PRIVATE KEY-----", "passphrase": null @@ -109,12 +109,12 @@ echo "$SSH_KEY_ID" **Response — `201 Created`** -`sshKeyId` is server-generated (UUID); `ownerId` is resolved from the caller's token, not a request field. Note `privateKey` and `passphrase` are absent from the response — they cannot leak through this endpoint. +`sshCredentialId` is server-generated (UUID); `ownerId` is resolved from the caller's token, not a request field. Note `privateKey` and `passphrase` are absent from the response — they cannot leak through this endpoint. ```json { - "sshKeyId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", - "sshKeyName": "expanse-key", + "sshCredentialId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", + "sshCredentialName": "expanse-key", "publicKey": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIExampleKeyDoNotUse airavata@expanse", "ownerId": "cilogon:12345" } @@ -124,15 +124,15 @@ echo "$SSH_KEY_ID" | Status | Cause | |---|---| -| `400 Bad Request` | `sshKeyName` or `publicKey` blank, or `privateKey` missing on create | +| `400 Bad Request` | `sshCredentialName` or `publicKey` blank, or `privateKey` missing on create | | `401 Unauthorized` | no token — every route here needs one, reads included | -| `403 Forbidden` | the key belongs to another user | -| `404 Not Found` | no such key, or the caller has no `users` row | -| `409 Conflict` | a cluster config or a data storage still presents the key | +| `403 Forbidden` | the credential belongs to another user | +| `404 Not Found` | no such credential, or the caller has no `users` row | +| `409 Conflict` | a cluster config or a data storage still presents the credential | **Validation errors — `400 Bad Request`** -Returned when `sshKeyName` or `publicKey` is blank. +Returned when `sshCredentialName` or `publicKey` is blank. ```json { @@ -140,7 +140,7 @@ Returned when `sshKeyName` or `publicKey` is blank. "error": "Bad Request", "message": "Validation failed", "fieldErrors": [ - { "field": "sshKeyName", "message": "SSH key name cannot be blank" }, + { "field": "sshCredentialName", "message": "SSH credential name cannot be blank" }, { "field": "publicKey", "message": "Public key cannot be blank" } ] } @@ -587,7 +587,7 @@ Every path is scoped by its cluster, so a `partitionId` belonging to one cluster ## Slurm Cluster Configs -A Slurm cluster config is one way of logging in to a [Slurm cluster](#slurm-clusters): an account, an SSH key, and the directory work is done beneath. The cluster says *where* jobs run; the config says *as whom*. +A Slurm cluster config is one way of logging in to a [Slurm cluster](#slurm-clusters): an account, an SSH credential, and the directory work is done beneath. The cluster says *where* jobs run; the config says *as whom*. Registering one is self-service — no admin authority — and it belongs to whoever registered it. Everyone else reaches it through its sharing rules, which is what lets a PI hand a team access to an allocation without handing over the key itself. Ownership is not transferable through the API: a run is launched against a config by id, so handing one over would silently hand over the identity every job on it submits as. @@ -600,7 +600,7 @@ Registering one is self-service — no admin authority — and it belongs to who `WRITE` deliberately stops short of control. A grantee may correct a work root; retiring the identity everybody else reaches the cluster through, or deciding who else gets it, stays with the owner. -The key is returned as its **public** summary only — name and public key. The private material has no field on any read model, so it cannot leak through these routes at all. +The credential is returned as its **public** summary only — name and public key. The private material has no field on any read model, so it cannot leak through these routes at all. ### Create Slurm Cluster Config @@ -608,7 +608,7 @@ The key is returned as its **public** summary only — name and public key. The POST /api/v1/slurm-cluster-configs ``` -Requires an authenticated principal with a `users` row. The owner is taken from the token — there is no owner field in the body. Uses `$CLUSTER_ID` from [Create Slurm Cluster](#create-slurm-cluster) and `$SSH_KEY_ID` from [Create SSH Key](#create-ssh-key) above, and captures `slurmClusterConfigId` into `$CLUSTER_CONFIG_ID` (requires `jq`). +Requires an authenticated principal with a `users` row. The owner is taken from the token — there is no owner field in the body. Uses `$CLUSTER_ID` from [Create Slurm Cluster](#create-slurm-cluster) and `$SSH_CREDENTIAL_ID` from [Create SSH Credential](#create-ssh-credential) above, and captures `slurmClusterConfigId` into `$CLUSTER_CONFIG_ID` (requires `jq`). ```bash CLUSTER_CONFIG_ID=$(curl -s -X POST localhost:9095/api/v1/slurm-cluster-configs \ @@ -620,7 +620,7 @@ CLUSTER_CONFIG_ID=$(curl -s -X POST localhost:9095/api/v1/slurm-cluster-configs "slurmClusterId": "'"$CLUSTER_ID"'", "loginUser": "airavata", "workRoot": "/expanse/lustre/scratch/airavata/jobs", - "sshKeyId": "'"$SSH_KEY_ID"'" + "sshCredentialId": "'"$SSH_CREDENTIAL_ID"'" }' | jq -r '.slurmClusterConfigId') ``` @@ -633,9 +633,9 @@ CLUSTER_CONFIG_ID=$(curl -s -X POST localhost:9095/api/v1/slurm-cluster-configs | `slurmClusterId` | string | required, must reference an existing [Slurm cluster](#create-slurm-cluster) | | `loginUser` | string | required, cannot be blank. The account on the head node | | `workRoot` | string | required, cannot be blank. Runs work beneath this directory | -| `sshKeyId` | string | required, must reference an existing [SSH key](#create-ssh-key) **the caller owns** — assigning someone else's key is `403` | +| `sshCredentialId` | string | required, must reference an existing [SSH credential](#create-ssh-credential) **the caller owns** — assigning someone else's credential is `403` | -A `slurmClusterId` or `sshKeyId` that does not resolve is `404 Not Found` rather than a dangling reference. +A `slurmClusterId` or `sshCredentialId` that does not resolve is `404 Not Found` rather than a dangling reference. **Response — `201 Created`** @@ -661,10 +661,10 @@ The cluster is inlined, since a config is only meaningful together with the mach }, "loginUser": "airavata", "workRoot": "/expanse/lustre/scratch/airavata/jobs", - "sshKeyId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", - "sshKey": { - "sshKeyId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", - "sshKeyName": "expanse-key", + "sshCredentialId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", + "sshCredential": { + "sshCredentialId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", + "sshCredentialName": "expanse-key", "publicKey": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIExampleKeyDoNotUse airavata@expanse", "ownerId": "cilogon:12345" }, @@ -692,7 +692,7 @@ DELETE /api/v1/slurm-cluster-configs/{slurmClusterConfigId} | Status | When | |---|---| | `403 Forbidden` | the config is not shared with the caller for what they asked to do; or a grantee tried to delete it or read its share list | -| `404 Not Found` | no such config, cluster or key | +| `404 Not Found` | no such config, cluster or credential | ### Share a Slurm Cluster Config @@ -1037,7 +1037,7 @@ Returned when e.g. `templateId` or `slurmRunSection` is blank, `defaultBatchJobC ## SCP Data Storages -An SCP data storage is a host and location that datasets are staged through. It names the host its data sits on, the account it is reached as, and the [SSH key](#create-ssh-key) presented for that account, and belongs to whoever registered it. A [data product](#data-products) names one as where its dataset lives, and a [data staging task](#create-data-staging-task) moves files between two of them. +An SCP data storage is a host and location that datasets are staged through. It names the host its data sits on, the account it is reached as, and the [SSH credential](#create-ssh-credential) presented for that account, and belongs to whoever registered it. A [data product](#data-products) names one as where its dataset lives, and a [data staging task](#create-data-staging-task) moves files between two of them. Registering one is documented up front, under [SCP Data Storage Registration](#scp-data-storage-registration), because the walkthrough needs one before it can register a dataset. What follows here is the rest of the resource. @@ -1069,7 +1069,7 @@ DELETE /api/v1/scp-data-storages/{dataStorageId}/shares/{sharingId} (owner) An SCP data storage is a host and location that datasets are staged through. Registering one is documented here, ahead of the resources that name it: [data products](#data-products) live on one, and a [data staging task](#create-data-staging-task) moves files between two. The rest of the resource — its routes, its sharing rules and how it is read back — is under [SCP Data Storages](#scp-data-storages) further down. -A storage spells out its own host and login account, so the only thing it needs registered ahead of it is the key it presents: [Create SSH Key](#create-ssh-key) above is the one call that comes before the one below. +A storage spells out its own host and login account, so the only thing it needs registered ahead of it is the key it presents: [Create SSH Credential](#create-ssh-credential) above is the one call that comes before the one below. ### Create SCP Data Storage @@ -1077,7 +1077,7 @@ A storage spells out its own host and login account, so the only thing it needs POST /api/v1/scp-data-storages ``` -Requires an authenticated principal with a `users` row. The owner is taken from the token — there is no owner field in the body — and is not transferable afterwards. Uses `$SSH_KEY_ID` from [Create SSH Key](#create-ssh-key) above, and captures `dataId` into `$STORAGE_ID` (requires `jq`) for the data product and staging steps further down. +Requires an authenticated principal with a `users` row. The owner is taken from the token — there is no owner field in the body — and is not transferable afterwards. Uses `$SSH_CREDENTIAL_ID` from [Create SSH Credential](#create-ssh-credential) above, and captures `dataId` into `$STORAGE_ID` (requires `jq`) for the data product and staging steps further down. A storage names both halves of where its data is: the **host** it sits on, as a host name and port, and the **account** it is reached as, as a login user and the key presented for it. Only the key is a reference, because its private half has to live somewhere it is never read back. Neither half is taken from an administrative catalogue: registering a storage is self-service, and having to ask an admin to enter the host first would defeat that. @@ -1090,7 +1090,7 @@ STORAGE_ID=$(curl -s -X POST localhost:9095/api/v1/scp-data-storages \ "hostName": "login.expanse.sdsc.edu", "port": 22, "loginUser": "airavata", - "sshKeyId": "'"$SSH_KEY_ID"'" + "sshCredentialId": "'"$SSH_CREDENTIAL_ID"'" }' | jq -r '.dataId') ``` @@ -1102,11 +1102,11 @@ STORAGE_ID=$(curl -s -X POST localhost:9095/api/v1/scp-data-storages \ | `hostName` | string | required, cannot be blank | | `port` | integer | optional, 1-65535, defaults to `22` | | `loginUser` | string | required, cannot be blank | -| `sshKeyId` | string | required, must reference an existing [SSH key](#create-ssh-key) **the caller owns** — assigning someone else's key is `403` | +| `sshCredentialId` | string | required, must reference an existing [SSH credential](#create-ssh-credential) **the caller owns** — assigning someone else's credential is `403` | **Response — `201 Created`** -The key is inlined as its safe (public-only) summary — the private material is never returned. `permission` is what the calling principal may do with the storage. +The credential is inlined as its safe (public-only) summary — the private material is never returned. `permission` is what the calling principal may do with the storage. ```json { @@ -1116,10 +1116,10 @@ The key is inlined as its safe (public-only) summary — the private material is "hostName": "login.expanse.sdsc.edu", "port": 22, "loginUser": "airavata", - "sshKeyId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", - "sshKey": { - "sshKeyId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", - "sshKeyName": "expanse-key", + "sshCredentialId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", + "sshCredential": { + "sshCredentialId": "8b1a9953-c461-4a3d-9d2f-0a1b2c3d4e5f", + "sshCredentialName": "expanse-key", "publicKey": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIExampleKeyDoNotUse airavata@expanse", "ownerId": "cilogon:12345" }, @@ -1235,7 +1235,7 @@ The routes above register *where* a dataset lives; these two move the bytes. The | `GET .../content` | `READ` on the product | | `PUT .../content` | `WRITE` on the product | -**The standing is on the product, and nothing else.** A product names a storage and that storage names an SSH key, but no standing on the storage is required and the key is never exposed: the platform reaches the host under the credential the storage owner registered, exactly as a staging task does. Requiring a storage share as well would make sharing a product useless on its own, and sharing the storage instead would hand over every dataset on that host rather than the one meant. +**The standing is on the product, and nothing else.** A product names a storage and that storage names an SSH credential, but no standing on the storage is required and the key is never exposed: the platform reaches the host under the credential the storage owner registered, exactly as a staging task does. Requiring a storage share as well would make sharing a product useless on its own, and sharing the storage instead would hand over every dataset on that host rather than the one meant. Only a **file** product can be transferred — `isFile: false` is `400`, because a directory moves as a tree and single-file SCP does not do that. @@ -1296,7 +1296,7 @@ A successful upload is what moves a product from `REGISTERD` to `PROVISIONED` | `400` | the product is a directory, is not on SCP storage, or the body could not be read | | `403` | no `READ` (download) or `WRITE` (upload) standing on the product | | `404` | no such product | -| `409` | the product names no path, or its storage is missing the host, login user or key | +| `409` | the product names no path, or its storage is missing the host, login user or credential | | `413` | the body ran past the upload cap | | `502` | the storage host could not be reached, or refused the transfer | @@ -1392,7 +1392,7 @@ are registered against a storage standing for the cluster's own filesystem. **The FASTA host.** A dataset on another host means a second storage, naming `data.lab.example.edu` rather than the cluster's login node. The account differs too: the group's login on the lab host is `labuser`, not the `airavata` the cluster is -reached as. It presents the *same* `$SSH_KEY_ID` as the first storage — one key can be +reached as. It presents the *same* `$SSH_CREDENTIAL_ID` as the first storage — one key can be authorised for as many accounts as its public half is installed on, and re-registering the key material would only give the same bytes a second id. `$FASTA_STORAGE_ID` is what the FASTA product names below, and it carries all three: the host, the account and the @@ -1407,7 +1407,7 @@ FASTA_STORAGE_ID=$(curl -s -X POST localhost:9095/api/v1/scp-data-storages \ "hostName": "data.lab.example.edu", "port": 22, "loginUser": "labuser", - "sshKeyId": "'"$SSH_KEY_ID"'" + "sshCredentialId": "'"$SSH_CREDENTIAL_ID"'" }' | jq -r '.dataId') ``` diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 834648f757..f027afaaad 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -42,15 +42,15 @@ info: Slurm clusters, partitions, application templates and batch deployments. Reads of the catalogue are open to anyone, including anonymous callers; writes require a role. - - **Ownership and sharing** guards the per-user resources — SSH keys, groups, - cluster configs, SCP data storages, data products and processes. Ownership + - **Ownership and sharing** guards the per-user resources — SSH credentials, + groups, cluster configs, SCP data storages, data products and processes. Ownership comes from the token and is never carried in a request body, so nothing can be created on someone else's behalf or handed over by editing it. None of these are readable anonymously. - SSH keys are the strictest case: platform admins are not treated as owners, and - there is no share that opens a key up. Private key material and passphrases are - write-only — no read endpoint has a field for them. + SSH credentials are the strictest case: platform admins are not treated as + owners, and there is no share that opens one up. Private key material and + passphrases are write-only — no read endpoint has a field for them. ## Errors @@ -77,7 +77,7 @@ tags: description: User accounts. - name: Groups description: Groups and their membership; the subject of a group share. - - name: SSH Keys + - name: SSH Credentials description: Registered keypairs. Private material is write-only. - name: Slurm Clusters description: The machine catalogue and its partitions. @@ -435,87 +435,87 @@ paths: "502": { $ref: "#/components/responses/BadGateway" } "500": { $ref: "#/components/responses/InternalError" } - # ------------------------------------------------------------- SSH keys + # ------------------------------------------------------------- SSH credentials - /api/v1/ssh-keys: + /api/v1/ssh-credentials: get: - tags: [SSH Keys] - summary: List the caller's SSH keys - description: Returns only the caller's own keys. Admins are not exempt. - operationId: listSSHKeys + tags: [SSH Credentials] + summary: List the caller's SSH credentials + description: Returns only the caller's own credentials. Admins are not exempt. + operationId: listSSHCredentials responses: "200": - description: The caller's keys. + description: The caller's credentials. content: application/json: schema: type: array - items: { $ref: "#/components/schemas/SSHKeyResponse" } + items: { $ref: "#/components/schemas/SSHCredentialResponse" } "401": { $ref: "#/components/responses/Unauthorized" } "403": { $ref: "#/components/responses/Forbidden" } "502": { $ref: "#/components/responses/BadGateway" } "500": { $ref: "#/components/responses/InternalError" } post: - tags: [SSH Keys] - summary: Register an SSH key + tags: [SSH Credentials] + summary: Register an SSH credential description: | Self-service for any authenticated caller with a `users` row. `privateKey` is required on create; `privateKey` and `passphrase` are write-only and are never returned by any read endpoint. - operationId: createSSHKey + operationId: createSSHCredential requestBody: required: true content: application/json: - schema: { $ref: "#/components/schemas/SSHKeyRequest" } + schema: { $ref: "#/components/schemas/SSHCredentialRequest" } responses: "201": - description: The registered key, without its private material. + description: The registered credential, without its private material. content: application/json: - schema: { $ref: "#/components/schemas/SSHKeyResponse" } + schema: { $ref: "#/components/schemas/SSHCredentialResponse" } "400": { $ref: "#/components/responses/BadRequest" } "401": { $ref: "#/components/responses/Unauthorized" } "403": { $ref: "#/components/responses/Forbidden" } "502": { $ref: "#/components/responses/BadGateway" } "500": { $ref: "#/components/responses/InternalError" } - /api/v1/ssh-keys/{sshKeyId}: + /api/v1/ssh-credentials/{sshCredentialId}: parameters: - - $ref: "#/components/parameters/SSHKeyId" + - $ref: "#/components/parameters/SSHCredentialId" get: - tags: [SSH Keys] - summary: Read an SSH key + tags: [SSH Credentials] + summary: Read an SSH credential description: Owner only. - operationId: getSSHKey + operationId: getSSHCredential responses: "200": - description: The key, without its private material. + description: The credential, without its private material. content: application/json: - schema: { $ref: "#/components/schemas/SSHKeyResponse" } + schema: { $ref: "#/components/schemas/SSHCredentialResponse" } "401": { $ref: "#/components/responses/Unauthorized" } "403": { $ref: "#/components/responses/Forbidden" } "404": { $ref: "#/components/responses/NotFound" } "502": { $ref: "#/components/responses/BadGateway" } "500": { $ref: "#/components/responses/InternalError" } put: - tags: [SSH Keys] - summary: Update an SSH key + tags: [SSH Credentials] + summary: Update an SSH credential description: | Owner only. An update that omits `privateKey` keeps the stored secret. - operationId: updateSSHKey + operationId: updateSSHCredential requestBody: required: true content: application/json: - schema: { $ref: "#/components/schemas/SSHKeyRequest" } + schema: { $ref: "#/components/schemas/SSHCredentialRequest" } responses: "200": - description: The updated key. + description: The updated credential. content: application/json: - schema: { $ref: "#/components/schemas/SSHKeyResponse" } + schema: { $ref: "#/components/schemas/SSHCredentialResponse" } "400": { $ref: "#/components/responses/BadRequest" } "401": { $ref: "#/components/responses/Unauthorized" } "403": { $ref: "#/components/responses/Forbidden" } @@ -523,12 +523,12 @@ paths: "502": { $ref: "#/components/responses/BadGateway" } "500": { $ref: "#/components/responses/InternalError" } delete: - tags: [SSH Keys] - summary: Delete an SSH key + tags: [SSH Credentials] + summary: Delete an SSH credential description: | - Owner only. A key still referenced by a cluster config or a data storage - cannot be deleted: `409 Conflict`. - operationId: deleteSSHKey + Owner only. A credential still referenced by a cluster config or a data + storage cannot be deleted: `409 Conflict`. + operationId: deleteSSHCredential responses: "204": { $ref: "#/components/responses/NoContent" } "401": { $ref: "#/components/responses/Unauthorized" } @@ -2443,8 +2443,8 @@ components: in: path required: true schema: { type: string } - SSHKeyId: - name: sshKeyId + SSHCredentialId: + name: sshCredentialId in: path required: true schema: { type: string, format: uuid } @@ -2553,8 +2553,8 @@ components: message: "Slurm cluster not found: c1d2e3f4-5678-4abc-9def-0123456789ab" Conflict: description: | - The request collides with existing state — deleting an SSH key still in use, - or a template that still has deployments. + The request collides with existing state — deleting an SSH credential still + in use, or a template that still has deployments. content: application/json: schema: { $ref: "#/components/schemas/Error" } @@ -2839,13 +2839,13 @@ components: # ---------------------------------------------------- Credentials - SSHKeyRequest: + SSHCredentialRequest: type: object description: | - The create/update payload for a key. `privateKey` is required on create; an + The create/update payload for a credential. `privateKey` is required on create; an update that omits it keeps the stored secret. There is no owner field. properties: - sshKeyName: { type: string } + sshCredentialName: { type: string } publicKey: { type: string } privateKey: type: [string, "null"] @@ -2853,20 +2853,20 @@ components: passphrase: type: [string, "null"] description: Write-only. Never returned by any read endpoint. - required: [sshKeyName, publicKey] + required: [sshCredentialName, publicKey] - SSHKeyResponse: + SSHCredentialResponse: type: object description: | - The read model for a key. It has no private key or passphrase field at all — + The read model for a credential. It has no private key or passphrase field at all — the secrets cannot leak through this endpoint because there is nowhere for them to go. properties: - sshKeyId: { type: string, format: uuid } - sshKeyName: { type: string } + sshCredentialId: { type: string, format: uuid } + sshCredentialName: { type: string } publicKey: { type: string } ownerId: { type: string } - required: [sshKeyId, sshKeyName, publicKey, ownerId] + required: [sshCredentialId, sshCredentialName, publicKey, ownerId] # -------------------------------------------------------- Compute @@ -2967,11 +2967,11 @@ components: slurmClusterId: { type: string, format: uuid } loginUser: { type: string } workRoot: { type: string } - sshKeyId: + sshCredentialId: type: string format: uuid - description: Must be a key the caller owns. - required: [slurmClusterId, loginUser, workRoot, sshKeyId] + description: Must be a credential the caller owns. + required: [slurmClusterId, loginUser, workRoot, sshCredentialId] SlurmClusterConfigResponse: type: object @@ -2990,10 +2990,10 @@ components: - { type: "null" } loginUser: { type: string } workRoot: { type: string } - sshKeyId: { type: [string, "null"], format: uuid } - sshKey: + sshCredentialId: { type: [string, "null"], format: uuid } + sshCredential: oneOf: - - { $ref: "#/components/schemas/SSHKeyResponse" } + - { $ref: "#/components/schemas/SSHCredentialResponse" } - { type: "null" } permission: type: string @@ -3147,11 +3147,11 @@ components: maximum: 65535 default: 22 loginUser: { type: string } - sshKeyId: + sshCredentialId: type: string format: uuid - description: Must be a key the caller owns. - required: [dataName, hostName, loginUser, sshKeyId] + description: Must be a credential the caller owns. + required: [dataName, hostName, loginUser, sshCredentialId] SCPDataStorageResponse: type: object @@ -3162,10 +3162,10 @@ components: hostName: { type: [string, "null"] } port: { type: [integer, "null"] } loginUser: { type: [string, "null"] } - sshKeyId: { type: [string, "null"], format: uuid } - sshKey: + sshCredentialId: { type: [string, "null"], format: uuid } + sshCredential: oneOf: - - { $ref: "#/components/schemas/SSHKeyResponse" } + - { $ref: "#/components/schemas/SSHCredentialResponse" } - { type: "null" } permission: type: string diff --git a/docs/sharing.md b/docs/sharing.md index 42cd691d26..61ca5d1913 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -15,7 +15,7 @@ Four kinds of record, each reached under its own collection: | Virtual data directory | `VIRTUAL_DATA_DIRECTORY` | `/api/v1/virtual-data-directories/{virtualDataDirectoryId}/shares` | | [Slurm cluster config](api.md#slurm-cluster-configs) | `SLURM_CLUSTER_CONFIG` | `/api/v1/slurm-cluster-configs/{slurmClusterConfigId}/shares` | -[SSH keys](api.md#ssh-keys) are deliberately **not** shareable. A key is the credential itself rather than something reached with one, so lending it out is what sharing a cluster config or a data storage is *for* — the key stays with its owner while other people submit and stage under it. +[SSH credentials](api.md#ssh-credentials) are deliberately **not** shareable. A credential *is* the secret material rather than something reached with it, so lending it out is what sharing a cluster config or a data storage is *for* — the credential stays with its owner while other people submit and stage under it. ## The model diff --git a/internal/app/app.go b/internal/app/app.go index 07e8d01d29..d20faf2b0e 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -68,7 +68,7 @@ type Services struct { GroupMember *iamsvc.GroupMemberService // Credentials. - SSHKey *credentialssvc.SSHKeyService + SSHCredential *credentialssvc.SSHCredentialService // Compute. SlurmCluster *computesvc.SlurmClusterService @@ -110,7 +110,7 @@ type Repositories struct { Users *iamrepo.UserRepository Groups *iamrepo.GroupRepository GroupMembers *iamrepo.GroupMemberRepository - SSHKeys *credentialsrepo.SSHKeyRepository + SSHCredentials *credentialsrepo.SSHCredentialRepository SlurmClusters *computerepo.SlurmClusterRepository ClusterPartitions *computerepo.ClusterPartitionRepository SlurmClusterConfigs *computerepo.SlurmClusterConfigRepository @@ -135,7 +135,7 @@ func NewRepositories(db *gorm.DB) *Repositories { Users: iamrepo.NewUserRepository(db), Groups: iamrepo.NewGroupRepository(db), GroupMembers: iamrepo.NewGroupMemberRepository(db), - SSHKeys: credentialsrepo.NewSSHKeyRepository(db), + SSHCredentials: credentialsrepo.NewSSHCredentialRepository(db), SlurmClusters: computerepo.NewSlurmClusterRepository(db), ClusterPartitions: computerepo.NewClusterPartitionRepository(db), SlurmClusterConfigs: computerepo.NewSlurmClusterConfigRepository(db), @@ -165,7 +165,7 @@ func NewServices(cfg config.Config, db *gorm.DB, repos *Repositories, workflowBa users := repos.Users groups := repos.Groups groupMembers := repos.GroupMembers - sshKeys := repos.SSHKeys + sshCredentials := repos.SSHCredentials clusters := repos.SlurmClusters partitions := repos.ClusterPartitions clusterConfigs := repos.SlurmClusterConfigs @@ -186,7 +186,7 @@ func NewServices(cfg config.Config, db *gorm.DB, repos *Repositories, workflowBa statusSvc := processsvc.NewStatusService(db, statuses, processes) configAccess := computesvc.NewConfigAccess(clusterConfigs, shares, groupMembers) - keyAccess := credentialssvc.NewKeyAccess(sshKeys) + keyAccess := credentialssvc.NewKeyAccess(sshCredentials) processSvs := processsvc.NewProcessService(db, processes, deployments, configAccess, users, statusSvc) batchDeploymentSvc := applicationsvc.NewBatchDeploymentService(db, deployments, templates, clusters) templateSvc := applicationsvc.NewTemplateService(db, templates, deployments) @@ -202,7 +202,7 @@ func NewServices(cfg config.Config, db *gorm.DB, repos *Repositories, workflowBa virtualDataDirectorySvc := datasvc.NewVirtualDataDirectoryService(db, virtualDirs, virtualFiles, shares, products, users, groupMembers) virtualDataFileSvc := datasvc.NewVirtualDataFileService(db, virtualDirs, virtualFiles, shares, products, groupMembers) virtualDataDirectorySharingSvc := datasvc.NewVirtualDataDirectorySharingService(db, virtualDirs, shares, groups, users, groupMembers) - sshKeySvc := credentialssvc.NewSSHKeyService(sshKeys, users, clusterConfigs, storages) + sshCredentialSvc := credentialssvc.NewSSHCredentialService(sshCredentials, users, clusterConfigs, storages) executionEngine := orchestration.NewExecutionEngine(stagingTasks, submissionTasks, monitoringTasks, storages, clusterConfigs, processes, deployments, templates, batchStatus, statuses, workflowBackend) @@ -218,7 +218,7 @@ func NewServices(cfg config.Config, db *gorm.DB, repos *Repositories, workflowBa // A key is deleted only when nothing presents it, and what can present one // lives in other verticals — hence both repositories here. - SSHKey: sshKeySvc, + SSHCredential: sshCredentialSvc, SlurmCluster: slurmClusterSvc, ClusterPartition: clusterPartitionSvc, @@ -247,7 +247,7 @@ func NewServices(cfg config.Config, db *gorm.DB, repos *Repositories, workflowBa Launch: processsvc.NewLaunchService(db, processSvs, batchDeploymentSvc, templateSvc, slurmClusterSvc, slurmClusterConfigSvc, - sshKeySvc, scpDataStorageSvc, dataProductSvc, statusSvc, stagingTasks, submissionTasks, + sshCredentialSvc, scpDataStorageSvc, dataProductSvc, statusSvc, stagingTasks, submissionTasks, monitoringTasks, commandTasks, executionEngine), ExecutionEngine: executionEngine, diff --git a/internal/db/foreignkey_test.go b/internal/db/foreignkey_test.go index 8bbf9df288..14d18e2f61 100644 --- a/internal/db/foreignkey_test.go +++ b/internal/db/foreignkey_test.go @@ -42,7 +42,7 @@ func TestForeignKeyDirections(t *testing.T) { } want := []fk{ {"user_roles", "user_id", "users", "user_id"}, - {"ssh_keys", "owner_id", "users", "user_id"}, + {"ssh_credentials", "owner_id", "users", "user_id"}, {"groups", "user_id", "users", "user_id"}, {"group_members", "group_id", "groups", "group_id"}, {"group_members", "user_id", "users", "user_id"}, @@ -51,9 +51,9 @@ func TestForeignKeyDirections(t *testing.T) { // carries its own headnode and data host, a storage its own host, and the // account reaching either belongs to a cluster config or the storage itself. {"slurm_cluster_configs", "slurm_cluster_id", "slurm_clusters", "slurm_cluster_id"}, - {"slurm_cluster_configs", "ssh_key_id", "ssh_keys", "ssh_key_id"}, + {"slurm_cluster_configs", "ssh_credential_id", "ssh_credentials", "ssh_credential_id"}, {"slurm_cluster_configs", "user_id", "users", "user_id"}, - {"scp_data_storages", "ssh_key_id", "ssh_keys", "ssh_key_id"}, + {"scp_data_storages", "ssh_credential_id", "ssh_credentials", "ssh_credential_id"}, {"scp_data_storages", "user_id", "users", "user_id"}, {"data_products", "user_id", "users", "user_id"}, // resource_sharings names its resource and its principal by (type, id) and so diff --git a/internal/db/migrate.go b/internal/db/migrate.go index 2e8eee3018..528b101e61 100644 --- a/internal/db/migrate.go +++ b/internal/db/migrate.go @@ -41,7 +41,7 @@ func Entities() []any { return []any{ // No outbound references. &iammodel.User{}, - &credentialsmodel.SSHKey{}, + &credentialsmodel.SSHCredential{}, &applicationmodel.Template{}, &applicationmodel.BatchJobConfig{}, diff --git a/internal/db/migrate_test.go b/internal/db/migrate_test.go index 03c6175087..9b6245fe7c 100644 --- a/internal/db/migrate_test.go +++ b/internal/db/migrate_test.go @@ -62,7 +62,7 @@ func TestAutoMigrateCreatesEveryTable(t *testing.T) { want := []string{ "users", "user_roles", "groups", "group_members", - "ssh_keys", + "ssh_credentials", "slurm_clusters", "cluster_partitions", "slurm_cluster_configs", "scp_data_storages", diff --git a/internal/db/migrations/0003_ssh_credential_rename.sql b/internal/db/migrations/0003_ssh_credential_rename.sql new file mode 100644 index 0000000000..754949f9d8 --- /dev/null +++ b/internal/db/migrations/0003_ssh_credential_rename.sql @@ -0,0 +1,53 @@ +-- Rename the SSH key entity to SSH credential, and record the two authentication +-- flags it gained. +-- +-- "Key" named the material; "credential" names what the platform actually holds and +-- presents. The distinction stopped being cosmetic once a row could carry a +-- certificate or demand a full interactive login instead of a bare keypair, which is +-- what is_certificate and full_interactive record. Those are the same booleans the +-- entity model declares; existing rows are plain keypairs, so both default to false +-- for the rewrite and the default is then dropped to match what AutoMigrate emits. +-- +-- Everything here is a rename, so no data moves and no id changes: a client holding a +-- credential id keeps addressing the same row. The names are the ones GORM derives +-- from the renamed Go identifiers, which is what keeps this file and AutoMigrate in +-- agreement — see TestBaselineMigrationMatchesAutoMigrate. +-- +-- This is a migration rather than an eighth re-recording of the baseline because 0002 +-- has already run: a database that applied it records version 2 and would skip a +-- rewritten 0001 entirely. + +ALTER TABLE "ssh_keys" RENAME TO "ssh_credentials"; + +ALTER TABLE "ssh_credentials" RENAME COLUMN "ssh_key_id" TO "ssh_credential_id"; + +ALTER TABLE "ssh_credentials" RENAME COLUMN "ssh_key_name" TO "ssh_credential_name"; + +ALTER TABLE "ssh_credentials" RENAME CONSTRAINT "fk_ssh_keys_owner" TO "fk_ssh_credentials_owner"; + +ALTER INDEX "ssh_keys_pkey" RENAME TO "ssh_credentials_pkey"; + +ALTER INDEX "idx_ssh_keys_owner_id" RENAME TO "idx_ssh_credentials_owner_id"; + +ALTER TABLE "ssh_credentials" ADD COLUMN "is_certificate" boolean NOT NULL DEFAULT false; + +ALTER TABLE "ssh_credentials" ALTER COLUMN "is_certificate" DROP DEFAULT; + +ALTER TABLE "ssh_credentials" ADD COLUMN "full_interactive" boolean NOT NULL DEFAULT false; + +ALTER TABLE "ssh_credentials" ALTER COLUMN "full_interactive" DROP DEFAULT; + +-- The two tables that present a credential. Neither reference changes target; only the +-- column and the names derived from it do. + +ALTER TABLE "scp_data_storages" RENAME COLUMN "ssh_key_id" TO "ssh_credential_id"; + +ALTER TABLE "scp_data_storages" RENAME CONSTRAINT "fk_scp_data_storages_ssh_key" TO "fk_scp_data_storages_ssh_credential"; + +ALTER INDEX "idx_scp_data_storages_ssh_key_id" RENAME TO "idx_scp_data_storages_ssh_credential_id"; + +ALTER TABLE "slurm_cluster_configs" RENAME COLUMN "ssh_key_id" TO "ssh_credential_id"; + +ALTER TABLE "slurm_cluster_configs" RENAME CONSTRAINT "fk_slurm_cluster_configs_ssh_key" TO "fk_slurm_cluster_configs_ssh_credential"; + +ALTER INDEX "idx_slurm_cluster_configs_ssh_key_id" RENAME TO "idx_slurm_cluster_configs_ssh_credential_id"; diff --git a/internal/httpx/errors.go b/internal/httpx/errors.go index 4b3308bbc8..d3c6561752 100644 --- a/internal/httpx/errors.go +++ b/internal/httpx/errors.go @@ -69,7 +69,7 @@ func BadRequest(format string, args ...any) *Error { } // Conflict reports a request that collides with existing state — the two cases here -// are deleting an SSH key still in use and deleting a template that still has +// are deleting an SSH credential still in use and deleting a template that still has // deployments. func Conflict(format string, args ...any) *Error { return &Error{Status: http.StatusConflict, Message: fmt.Sprintf(format, args...)} diff --git a/internal/orchestration/batch.go b/internal/orchestration/batch.go index 5675f4f931..73e87bb52c 100644 --- a/internal/orchestration/batch.go +++ b/internal/orchestration/batch.go @@ -32,6 +32,7 @@ import ( applicationmodel "github.com/apache/airavata/api/application/model" model "github.com/apache/airavata/api/process/model" + ssh "github.com/apache/airavata/internal/ssh" ) type SubmitBatchJobParameters struct { @@ -88,8 +89,8 @@ func (a *ExecutionEngine) submitBatchJob(ctx context.Context, executionContext * } slog.Info("Creating job working directory through SSH", "taskId", taskID, "processId", processID, "destinationDirectory", *jst.WorkingDir) - if err := CreateDirectorySSH(ctx, clusterConfig.SlurmCluster.HeadnodeHost, clusterConfig.SlurmCluster.HeadnodePort, - clusterConfig.LoginUser, *clusterConfig.SSHKey, *jst.WorkingDir); err != nil { + if err := ssh.CreateDirectorySSH(ctx, clusterConfig.SlurmCluster.HeadnodeHost, clusterConfig.SlurmCluster.HeadnodePort, + clusterConfig.LoginUser, *clusterConfig.SSHCredential, *jst.WorkingDir); err != nil { slog.Error("Failed to create destination directory through SSH", "taskId", taskID, "processId", processID, "destinationDirectory", *jst.WorkingDir, "error", err) return nil, err @@ -97,9 +98,9 @@ func (a *ExecutionEngine) submitBatchJob(ctx context.Context, executionContext * scriptUploadPath := path.Join(*jst.WorkingDir, "script.slurm") slog.Info("Uploading slurm script to cluster", "taskId", taskID, "processId", processID, "scriptPath", scriptPath, "remotePath", scriptUploadPath) - if err := UploadFileToSCP(ctx, clusterConfig.SlurmCluster.HeadnodeHost, + if err := ssh.UploadFileToSCP(ctx, clusterConfig.SlurmCluster.HeadnodeHost, clusterConfig.SlurmCluster.HeadnodePort, clusterConfig.LoginUser, - *clusterConfig.SSHKey, scriptPath, scriptUploadPath); err != nil { + *clusterConfig.SSHCredential, scriptPath, scriptUploadPath); err != nil { slog.Error("Failed to upload slurm script to cluster", "taskId", taskID, "processId", processID, "remotePath", scriptUploadPath, "error", err) return nil, err } @@ -116,11 +117,11 @@ func (a *ExecutionEngine) submitBatchJob(ctx context.Context, executionContext * // sbatch is run from the working directory so that a script writing relative paths — // and the job's own stdout and stderr files — land beside the staged inputs rather // than in the login user's home. - submitCommand := "cd " + shellQuote(*jst.WorkingDir) + " && " + shellQuote(sbatchPath) + " " + shellQuote(scriptUploadPath) + submitCommand := "cd " + ssh.ShellQuote(*jst.WorkingDir) + " && " + ssh.ShellQuote(sbatchPath) + " " + ssh.ShellQuote(scriptUploadPath) - stdout, stderr, err := runSSHCommand(ctx, clusterConfig.SlurmCluster.HeadnodeHost, + stdout, stderr, err := ssh.RunSSHCommand(ctx, clusterConfig.SlurmCluster.HeadnodeHost, clusterConfig.SlurmCluster.HeadnodePort, clusterConfig.LoginUser, - *clusterConfig.SSHKey, submitCommand, + *clusterConfig.SSHCredential, submitCommand, fmt.Sprintf("ssh sbatch %s@%s:%s", clusterConfig.LoginUser, clusterConfig.SlurmCluster.HeadnodeHost, scriptUploadPath)) if err != nil { slog.Error("Failed to submit slurm script to cluster", "taskId", taskID, "processId", processID, "remotePath", scriptUploadPath, "stdout", stdout, "stderr", stderr, "error", err) diff --git a/internal/orchestration/data.go b/internal/orchestration/data.go index 8bf081b8d0..f4accf8218 100644 --- a/internal/orchestration/data.go +++ b/internal/orchestration/data.go @@ -29,14 +29,9 @@ import ( "path/filepath" datamodel "github.com/apache/airavata/api/data/model" + ssh "github.com/apache/airavata/internal/ssh" ) -type FileMetadata struct { - Path string - Size int64 - Mode os.FileMode -} - func (a *ExecutionEngine) copyData(ctx context.Context, executionContext *ExecutionContext, processId string, dataStagingTaskId string) (*ExecutionContext, error) { dataStagingTask, err := a.dataStagingTasks.FindByIDAndProcessID(ctx, dataStagingTaskId, processId) if err != nil { @@ -54,7 +49,7 @@ func (a *ExecutionEngine) copyData(ctx context.Context, executionContext *Execut return nil, err } - err = DownloadFileFromSCP(ctx, *s.HostName, *s.Port, *s.LoginUser, *s.SSHKey, + err = ssh.DownloadFileFromSCP(ctx, *s.HostName, *s.Port, *s.LoginUser, *s.SSHCredential, *dataStagingTask.SourcePath, localTempDataPath) if err != nil { slog.Error("Failed to download file through SCP", "taskId", dataStagingTask.ID, "processId", *dataStagingTask.ProcessID, @@ -71,8 +66,8 @@ func (a *ExecutionEngine) copyData(ctx context.Context, executionContext *Execut return nil, err } - err = DownloadFileFromSCP(ctx, s.SlurmCluster.HeadnodeHost, s.SlurmCluster.HeadnodePort, s.LoginUser, - *s.SSHKey, *dataStagingTask.SourcePath, localTempDataPath) + err = ssh.DownloadFileFromSCP(ctx, s.SlurmCluster.HeadnodeHost, s.SlurmCluster.HeadnodePort, s.LoginUser, + *s.SSHCredential, *dataStagingTask.SourcePath, localTempDataPath) if err != nil { slog.Error("Failed to download file from HPC", "taskId", dataStagingTask.ID, "processId", *dataStagingTask.ProcessID, "sourcePath", *dataStagingTask.SourcePath, "destinationPath", localTempDataPath, "error", err) @@ -89,13 +84,13 @@ func (a *ExecutionEngine) copyData(ctx context.Context, executionContext *Execut } // Ensure the destination directory exists before attempting the SCP upload. - if err := CreateDirectorySSH(ctx, *s.HostName, *s.Port, *s.LoginUser, *s.SSHKey, filepath.Dir(*dataStagingTask.DestinationPath)); err != nil { + if err := ssh.CreateDirectorySSH(ctx, *s.HostName, *s.Port, *s.LoginUser, *s.SSHCredential, filepath.Dir(*dataStagingTask.DestinationPath)); err != nil { slog.Error("Failed to create destination directory through SSH", "taskId", dataStagingTask.ID, "processId", *dataStagingTask.ProcessID, "destinationDirectory", filepath.Dir(*dataStagingTask.DestinationPath), "error", err) return nil, err } - err = UploadFileToSCP(ctx, *s.HostName, *s.Port, *s.LoginUser, *s.SSHKey, localTempDataPath, *dataStagingTask.DestinationPath) + err = ssh.UploadFileToSCP(ctx, *s.HostName, *s.Port, *s.LoginUser, *s.SSHCredential, localTempDataPath, *dataStagingTask.DestinationPath) if err != nil { slog.Error("Failed to upload file through SCP", "taskId", dataStagingTask.ID, "processId", *dataStagingTask.ProcessID, "sourcePath", localTempDataPath, "destinationPath", *dataStagingTask.DestinationPath, "error", err) @@ -113,15 +108,15 @@ func (a *ExecutionEngine) copyData(ctx context.Context, executionContext *Execut } // Ensure the destination directory exists before attempting the SCP upload. - if err := CreateDirectorySSH(ctx, s.SlurmCluster.HeadnodeHost, s.SlurmCluster.HeadnodePort, s.LoginUser, *s.SSHKey, + if err := ssh.CreateDirectorySSH(ctx, s.SlurmCluster.HeadnodeHost, s.SlurmCluster.HeadnodePort, s.LoginUser, *s.SSHCredential, filepath.Dir(*dataStagingTask.DestinationPath)); err != nil { slog.Error("Failed to create destination directory on HPC through SSH", "taskId", dataStagingTask.ID, "processId", *dataStagingTask.ProcessID, "destinationDirectory", filepath.Dir(*dataStagingTask.DestinationPath), "error", err) return nil, err } - err = UploadFileToSCP(ctx, s.SlurmCluster.HeadnodeHost, s.SlurmCluster.HeadnodePort, s.LoginUser, - *s.SSHKey, localTempDataPath, *dataStagingTask.DestinationPath) + err = ssh.UploadFileToSCP(ctx, s.SlurmCluster.HeadnodeHost, s.SlurmCluster.HeadnodePort, s.LoginUser, + *s.SSHCredential, localTempDataPath, *dataStagingTask.DestinationPath) if err != nil { slog.Error("Failed to upload file to HPC", "taskId", dataStagingTask.ID, "processId", *dataStagingTask.ProcessID, "sourcePath", localTempDataPath, "destinationPath", *dataStagingTask.DestinationPath, "error", err) diff --git a/internal/orchestration/slurm.go b/internal/orchestration/slurm.go index a42c051ea0..927c75176a 100644 --- a/internal/orchestration/slurm.go +++ b/internal/orchestration/slurm.go @@ -32,6 +32,7 @@ import ( appmodel "github.com/apache/airavata/api/application/model" computemodel "github.com/apache/airavata/api/compute/model" model "github.com/apache/airavata/api/process/model" + ssh "github.com/apache/airavata/internal/ssh" ) const slurmScriptTemplate = `#!/bin/bash @@ -136,7 +137,7 @@ func buildSlurmScript( // The same directory as a shell word. An SBATCH directive is not shell and must // carry the path bare, but the cd in the body is, and a work root with a space // in it would otherwise become two arguments. - "work_dir_quoted": shellQuote(workDir), + "work_dir_quoted": ssh.ShellQuote(workDir), "stdout_file": path.Join(workDir, "stdout.txt"), "stderr_file": path.Join(workDir, "stderr.txt"), "wall_time": slurmWallTime(jobConfig.WallTimeMinutes), diff --git a/internal/server/server.go b/internal/server/server.go index 51994f1520..7fb389b741 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -49,7 +49,7 @@ func New(cfg config.Config, svcs *app.Services, introspector auth.Introspector) iamctl.NewUserController(svcs.User).Register(mux) iamctl.NewGroupController(svcs.Group).Register(mux) iamctl.NewGroupMemberController(svcs.GroupMember).Register(mux) - credentialsctl.NewSSHKeyController(svcs.SSHKey).Register(mux) + credentialsctl.NewSSHCredentialController(svcs.SSHCredential).Register(mux) computectl.NewSlurmClusterController(svcs.SlurmCluster).Register(mux) computectl.NewClusterPartitionController(svcs.ClusterPartition).Register(mux) computectl.NewSlurmClusterConfigController(svcs.SlurmClusterConfig).Register(mux) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 239a5b83ae..2460a557d3 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -215,22 +215,22 @@ func (h *harness) seedCluster(name string) string { func (h *harness) seedClusterConfig(token, name string) (configID, clusterID string) { h.t.Helper() clusterID = h.seedCluster(name) - keyID := h.seedSSHKey(name, token) + keyID := h.seedSSHCredential(name, token) out := h.mustDo(http.MethodPost, "/api/v1/slurm-cluster-configs", token, map[string]any{ "name": name, "slurmClusterId": clusterID, - "loginUser": "runner", "workRoot": "/scratch/runner", "sshKeyId": keyID, + "loginUser": "runner", "workRoot": "/scratch/runner", "sshCredentialId": keyID, }, http.StatusCreated) return out["slurmClusterConfigId"].(string), clusterID } -// seedSSHKey registers a key owned by the caller of token and returns its id. The +// seedSSHCredential registers a key owned by the caller of token and returns its id. The // owner matters: only they may put it on a cluster config or a data storage. -func (h *harness) seedSSHKey(name, token string) string { +func (h *harness) seedSSHCredential(name, token string) string { h.t.Helper() - key := h.mustDo(http.MethodPost, "/api/v1/ssh-keys", token, map[string]any{ - "sshKeyName": name, "publicKey": "ssh-ed25519 AAAA", "privateKey": "PRIVATE", + key := h.mustDo(http.MethodPost, "/api/v1/ssh-credentials", token, map[string]any{ + "sshCredentialName": name, "publicKey": "ssh-ed25519 AAAA", "privateKey": "PRIVATE", }, http.StatusCreated) - return key["sshKeyId"].(string) + return key["sshCredentialId"].(string) } func TestHealthIsOpen(t *testing.T) { @@ -344,22 +344,22 @@ func TestDeletingClusterCascadesToPartitions(t *testing.T) { } // The private key must never appear in a response, and creation must insist on one. -func TestSSHKeySecretsAreNeverReturnedAndRequiredOnCreate(t *testing.T) { +func TestSSHCredentialSecretsAreNeverReturnedAndRequiredOnCreate(t *testing.T) { h := newHarness(t) - rec := h.do(http.MethodPost, "/api/v1/ssh-keys", tokenAdmin, map[string]any{ - "sshKeyName": "k", "publicKey": "ssh-ed25519 AAAA", + rec := h.do(http.MethodPost, "/api/v1/ssh-credentials", tokenAdmin, map[string]any{ + "sshCredentialName": "k", "publicKey": "ssh-ed25519 AAAA", }) if rec.Code != http.StatusBadRequest { t.Fatalf("create without a private key: status = %d, want 400", rec.Code) } - created := h.mustDo(http.MethodPost, "/api/v1/ssh-keys", tokenAdmin, map[string]any{ - "sshKeyName": "k", "publicKey": "ssh-ed25519 AAAA", "privateKey": "SUPER-SECRET", + created := h.mustDo(http.MethodPost, "/api/v1/ssh-credentials", tokenAdmin, map[string]any{ + "sshCredentialName": "k", "publicKey": "ssh-ed25519 AAAA", "privateKey": "SUPER-SECRET", "passphrase": "hunter2", }, http.StatusCreated) - for _, path := range []string{"/api/v1/ssh-keys", "/api/v1/ssh-keys/" + created["sshKeyId"].(string)} { + for _, path := range []string{"/api/v1/ssh-credentials", "/api/v1/ssh-credentials/" + created["sshCredentialId"].(string)} { body := h.do(http.MethodGet, path, tokenAdmin, nil).Body.String() if strings.Contains(body, "SUPER-SECRET") || strings.Contains(body, "hunter2") { t.Errorf("GET %s leaked a secret: %s", path, body) @@ -369,25 +369,25 @@ func TestSSHKeySecretsAreNeverReturnedAndRequiredOnCreate(t *testing.T) { // A client round-tripping a response has no private key to send back, so a blank one // must mean "leave it alone" rather than "erase it". -func TestUpdatingSSHKeyWithoutSecretsPreservesThem(t *testing.T) { +func TestUpdatingSSHCredentialWithoutSecretsPreservesThem(t *testing.T) { h := newHarness(t) - created := h.mustDo(http.MethodPost, "/api/v1/ssh-keys", tokenAdmin, map[string]any{ - "sshKeyName": "k", "publicKey": "ssh-ed25519 AAAA", "privateKey": "ORIGINAL", + created := h.mustDo(http.MethodPost, "/api/v1/ssh-credentials", tokenAdmin, map[string]any{ + "sshCredentialName": "k", "publicKey": "ssh-ed25519 AAAA", "privateKey": "ORIGINAL", "passphrase": "original-pass", }, http.StatusCreated) - keyID := created["sshKeyId"].(string) + keyID := created["sshCredentialId"].(string) - h.mustDo(http.MethodPut, "/api/v1/ssh-keys/"+keyID, tokenAdmin, map[string]any{ - "sshKeyName": "renamed", "publicKey": "ssh-ed25519 BBBB", "privateKey": "", + h.mustDo(http.MethodPut, "/api/v1/ssh-credentials/"+keyID, tokenAdmin, map[string]any{ + "sshCredentialName": "renamed", "publicKey": "ssh-ed25519 BBBB", "privateKey": "", }, http.StatusOK) var stored struct { - PrivateKey string - Passphrase *string - SSHKeyName string + PrivateKey string + Passphrase *string + SSHCredentialName string } - h.db.Table("ssh_keys").Select("private_key, passphrase, ssh_key_name"). - Where("ssh_key_id = ?", keyID).Scan(&stored) + h.db.Table("ssh_credentials").Select("private_key, passphrase, ssh_credential_name"). + Where("ssh_credential_id = ?", keyID).Scan(&stored) if stored.PrivateKey != "ORIGINAL" { t.Errorf("private key = %q, want it preserved across an update that omitted it", stored.PrivateKey) @@ -395,25 +395,25 @@ func TestUpdatingSSHKeyWithoutSecretsPreservesThem(t *testing.T) { if stored.Passphrase == nil || *stored.Passphrase != "original-pass" { t.Errorf("passphrase = %v, want it preserved", stored.Passphrase) } - if stored.SSHKeyName != "renamed" { - t.Errorf("name = %q, want the update applied", stored.SSHKeyName) + if stored.SSHCredentialName != "renamed" { + t.Errorf("name = %q, want the update applied", stored.SSHCredentialName) } } // A key belongs to whoever registered it and to nobody else. There is no share to open // one up, and platform admins get no standing on one either — reading or repointing // someone's key material is not an administrative act. -func TestSSHKeyIsPrivateToItsOwner(t *testing.T) { +func TestSSHCredentialIsPrivateToItsOwner(t *testing.T) { h := newHarness(t) - keyID := h.seedSSHKey("alices-key", tokenAlice) - base := "/api/v1/ssh-keys/" + keyID + keyID := h.seedSSHCredential("alices-key", tokenAlice) + base := "/api/v1/ssh-credentials/" + keyID created := h.mustDo(http.MethodGet, base, tokenAlice, nil, http.StatusOK) if created["ownerId"] != "alice" { t.Errorf("ownerId = %v, want it taken from the token", created["ownerId"]) } - update := map[string]any{"sshKeyName": "renamed", "publicKey": "ssh-ed25519 AAAA"} + update := map[string]any{"sshCredentialName": "renamed", "publicKey": "ssh-ed25519 AAAA"} for _, token := range []string{tokenBob, tokenAdmin} { if rec := h.do(http.MethodGet, base, token, nil); rec.Code != http.StatusForbidden { t.Errorf("read by %s: status = %d, want 403", token, rec.Code) @@ -428,15 +428,15 @@ func TestSSHKeyIsPrivateToItsOwner(t *testing.T) { h.mustDo(http.MethodPut, base, tokenAlice, update, http.StatusOK) // Listing is the caller's own keys, so it never becomes a way around the above. - if mine := h.list("/api/v1/ssh-keys", tokenAlice); len(mine) != 1 { + if mine := h.list("/api/v1/ssh-credentials", tokenAlice); len(mine) != 1 { t.Errorf("alice's keys = %d, want 1", len(mine)) } for _, token := range []string{tokenBob, tokenAdmin} { - if theirs := h.list("/api/v1/ssh-keys", token); len(theirs) != 0 { + if theirs := h.list("/api/v1/ssh-credentials", token); len(theirs) != 0 { t.Errorf("%s sees %d of alice's keys, want 0", token, len(theirs)) } } - if rec := h.do(http.MethodGet, "/api/v1/ssh-keys", "", nil); rec.Code != http.StatusUnauthorized { + if rec := h.do(http.MethodGet, "/api/v1/ssh-credentials", "", nil); rec.Code != http.StatusUnauthorized { t.Errorf("anonymous list: status = %d, want 401", rec.Code) } } @@ -445,19 +445,19 @@ func TestSSHKeyIsPrivateToItsOwner(t *testing.T) { // under a name of the caller's choosing, so it is refused unless they own the key. func TestAssigningAKeyRequiresOwningIt(t *testing.T) { h := newHarness(t) - aliceKey := h.seedSSHKey("alices-key", tokenAlice) + aliceKey := h.seedSSHCredential("alices-key", tokenAlice) clusterID := h.seedCluster("shared-machine") config := map[string]any{ "slurmClusterId": clusterID, "loginUser": "bob", - "workRoot": "/scratch/bob", "sshKeyId": aliceKey, + "workRoot": "/scratch/bob", "sshCredentialId": aliceKey, } if rec := h.do(http.MethodPost, "/api/v1/slurm-cluster-configs", tokenBob, config); rec.Code != http.StatusForbidden { t.Errorf("config on another user's key: status = %d, want 403", rec.Code) } storage := map[string]any{ "dataName": "bobs-scratch", "hostName": "data.example.edu", - "loginUser": "bob", "sshKeyId": aliceKey, + "loginUser": "bob", "sshCredentialId": aliceKey, } if rec := h.do(http.MethodPost, "/api/v1/scp-data-storages", tokenBob, storage); rec.Code != http.StatusForbidden { t.Errorf("storage on another user's key: status = %d, want 403", rec.Code) @@ -469,8 +469,8 @@ func TestAssigningAKeyRequiresOwningIt(t *testing.T) { } // With his own key, both go through. - bobKey := h.seedSSHKey("bobs-key", tokenBob) - config["sshKeyId"], storage["sshKeyId"] = bobKey, bobKey + bobKey := h.seedSSHCredential("bobs-key", tokenBob) + config["sshCredentialId"], storage["sshCredentialId"] = bobKey, bobKey h.mustDo(http.MethodPost, "/api/v1/slurm-cluster-configs", tokenBob, config, http.StatusCreated) h.mustDo(http.MethodPost, "/api/v1/scp-data-storages", tokenBob, storage, http.StatusCreated) } @@ -487,7 +487,7 @@ func TestGranteeMayEditAroundAKeyTheyDoNotOwn(t *testing.T) { body := func(keyID string) map[string]any { return map[string]any{ "dataName": "renamed", "hostName": "shared-staging.example.edu", - "loginUser": "runner", "sshKeyId": keyID, + "loginUser": "runner", "sshCredentialId": keyID, } } @@ -496,49 +496,49 @@ func TestGranteeMayEditAroundAKeyTheyDoNotOwn(t *testing.T) { h.mustDo(http.MethodPut, base, tokenBob, body(aliceKey), http.StatusOK) // Putting a third party's key on it is refused — bob owns neither. - adminKey := h.seedSSHKey("admins-key", tokenAdmin) + adminKey := h.seedSSHCredential("admins-key", tokenAdmin) if rec := h.do(http.MethodPut, base, tokenBob, body(adminKey)); rec.Code != http.StatusForbidden { t.Errorf("grantee assigning a third party's key: status = %d, want 403", rec.Code) } // His own key he may lend: assigning is allowed exactly when the caller owns the // key, so after the swap the storage stages under bob's. - bobKey := h.seedSSHKey("bobs-key", tokenBob) + bobKey := h.seedSSHCredential("bobs-key", tokenBob) updated := h.mustDo(http.MethodPut, base, tokenBob, body(bobKey), http.StatusOK) - if updated["sshKeyId"] != bobKey { - t.Errorf("sshKeyId = %v, want bob's key %s", updated["sshKeyId"], bobKey) + if updated["sshCredentialId"] != bobKey { + t.Errorf("sshCredentialId = %v, want bob's key %s", updated["sshCredentialId"], bobKey) } } // Nothing in the schema stops a key from being deleted out from under a credential, // so the service has to. -func TestDeletingSSHKeyInUseConflicts(t *testing.T) { +func TestDeletingSSHCredentialInUseConflicts(t *testing.T) { h := newHarness(t) // Held by a cluster config: what a run logs in with. - configKey := h.seedSSHKey("config-key", tokenAlice) + configKey := h.seedSSHCredential("config-key", tokenAlice) clusterID := h.seedCluster("in-use") h.mustDo(http.MethodPost, "/api/v1/slurm-cluster-configs", tokenAlice, map[string]any{ "slurmClusterId": clusterID, "loginUser": "runner", - "workRoot": "/scratch/runner", "sshKeyId": configKey, + "workRoot": "/scratch/runner", "sshCredentialId": configKey, }, http.StatusCreated) - if rec := h.do(http.MethodDelete, "/api/v1/ssh-keys/"+configKey, tokenAlice, nil); rec.Code != http.StatusConflict { + if rec := h.do(http.MethodDelete, "/api/v1/ssh-credentials/"+configKey, tokenAlice, nil); rec.Code != http.StatusConflict { t.Errorf("delete of a key a config presents: status = %d, want 409", rec.Code) } // Held by a data storage: what a dataset is staged under. - storageKey := h.seedSSHKey("storage-key", tokenAlice) + storageKey := h.seedSSHCredential("storage-key", tokenAlice) h.mustDo(http.MethodPost, "/api/v1/scp-data-storages", tokenAlice, map[string]any{ "dataName": "in-use", "hostName": "in-use.example.edu", - "loginUser": "runner", "sshKeyId": storageKey, + "loginUser": "runner", "sshCredentialId": storageKey, }, http.StatusCreated) - if rec := h.do(http.MethodDelete, "/api/v1/ssh-keys/"+storageKey, tokenAlice, nil); rec.Code != http.StatusConflict { + if rec := h.do(http.MethodDelete, "/api/v1/ssh-credentials/"+storageKey, tokenAlice, nil); rec.Code != http.StatusConflict { t.Errorf("delete of a key a storage presents: status = %d, want 409", rec.Code) } // Presented by nothing, it goes. - freeKey := h.seedSSHKey("free-key", tokenAlice) - h.mustDo(http.MethodDelete, "/api/v1/ssh-keys/"+freeKey, tokenAlice, nil, http.StatusNoContent) + freeKey := h.seedSSHCredential("free-key", tokenAlice) + h.mustDo(http.MethodDelete, "/api/v1/ssh-credentials/"+freeKey, tokenAlice, nil, http.StatusNoContent) } func TestDeletingTemplateWithDeploymentsConflicts(t *testing.T) { @@ -765,13 +765,13 @@ func TestStrongestShareWins(t *testing.T) { } // seedStorage registers an SCP data storage owned by the caller of token, returning the -// SSH key it stages under along with its id. +// SSH credential it stages under along with its id. func (h *harness) seedStorage(name, token string) (keyID, storageID string) { h.t.Helper() - keyID = h.seedSSHKey(name, token) + keyID = h.seedSSHCredential(name, token) out := h.mustDo(http.MethodPost, "/api/v1/scp-data-storages", token, map[string]any{ "dataName": name, "hostName": name + ".example.edu", - "loginUser": "runner", "sshKeyId": keyID, + "loginUser": "runner", "sshCredentialId": keyID, }, http.StatusCreated) return keyID, out["dataId"].(string) } @@ -826,8 +826,8 @@ func TestStorageIsReachableOnlyByOwnerAndGrantees(t *testing.T) { if created["port"].(float64) != 22 { t.Errorf("port = %v, want the default 22 rather than the zero value", created["port"]) } - if created["sshKey"] == nil { - t.Error("storage response did not inline its SSH key") + if created["sshCredential"] == nil { + t.Error("storage response did not inline its SSH credential") } if rec := h.do(http.MethodGet, base, tokenBob, nil); rec.Code != http.StatusForbidden { @@ -860,7 +860,7 @@ func TestStorageIsReachableOnlyByOwnerAndGrantees(t *testing.T) { // READ is not WRITE, and no share confers control. repoint := map[string]any{ "dataName": "renamed", "hostName": "staging.example.edu", - "loginUser": "runner", "sshKeyId": keyID, + "loginUser": "runner", "sshCredentialId": keyID, } if rec := h.do(http.MethodPut, base, tokenBob, repoint); rec.Code != http.StatusForbidden { t.Errorf("update with READ: status = %d, want 403", rec.Code) @@ -895,11 +895,11 @@ func TestStorageIsReachableOnlyByOwnerAndGrantees(t *testing.T) { // rather than a storage pointing at a key that does not exist. func TestStorageRegistrationValidatesItsHostAndKey(t *testing.T) { h := newHarness(t) - keyID := h.seedSSHKey("validation", tokenAlice) + keyID := h.seedSSHCredential("validation", tokenAlice) body := func(over map[string]any) map[string]any { out := map[string]any{ "dataName": "scratch", "hostName": "login.example.edu", - "loginUser": "runner", "sshKeyId": keyID, + "loginUser": "runner", "sshCredentialId": keyID, } for k, v := range over { out[k] = v @@ -922,8 +922,8 @@ func TestStorageRegistrationValidatesItsHostAndKey(t *testing.T) { t.Errorf("out-of-range port: status = %d, want 400", rec.Code) } if rec := h.do(http.MethodPost, "/api/v1/scp-data-storages", tokenAlice, - body(map[string]any{"sshKeyId": "nope"})); rec.Code != http.StatusNotFound { - t.Errorf("unknown SSH key: status = %d, want 404", rec.Code) + body(map[string]any{"sshCredentialId": "nope"})); rec.Code != http.StatusNotFound { + t.Errorf("unknown SSH credential: status = %d, want 404", rec.Code) } } @@ -1996,7 +1996,7 @@ func TestGetProcessStatusIsScopedToItsProcess(t *testing.T) { func TestUnregisteredPrincipalCannotOwnResources(t *testing.T) { h := newHarness(t) clusterID := h.seedCluster("ghost") - keyID := h.seedSSHKey("ghost", tokenAlice) + keyID := h.seedSSHCredential("ghost", tokenAlice) // A token naming a principal with no users row. introspector := stubIntrospector{ @@ -2006,7 +2006,7 @@ func TestUnregisteredPrincipalCannotOwnResources(t *testing.T) { rec := h.do(http.MethodPost, "/api/v1/slurm-cluster-configs", "ghost-token", map[string]any{ "slurmClusterId": clusterID, "loginUser": "ghost", - "workRoot": "/scratch/ghost", "sshKeyId": keyID, + "workRoot": "/scratch/ghost", "sshCredentialId": keyID, }) if rec.Code != http.StatusNotFound { t.Errorf("status = %d, want 404\nbody: %s", rec.Code, rec.Body.String()) @@ -2308,7 +2308,7 @@ func TestUnknownResourcesReportNotFound(t *testing.T) { for _, path := range []string{ "/api/v1/slurm-clusters/nope", "/api/v1/slurm-cluster-configs/nope", - "/api/v1/ssh-keys/nope", + "/api/v1/ssh-credentials/nope", "/api/v1/application-templates/nope", "/api/v1/slurm-deployments/nope", "/api/v1/processes/nope", diff --git a/internal/ssh/operations.go b/internal/ssh/operations.go new file mode 100644 index 0000000000..e4aac9784a --- /dev/null +++ b/internal/ssh/operations.go @@ -0,0 +1,123 @@ +package ssh + +import ( + "context" + "fmt" + credmodel "github.com/apache/airavata/api/credentials/model" + "log/slog" + "os" + "path/filepath" + "strings" +) + +// Copies a single remote file to localPath over SSH +func DownloadFileFromSCP(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential, remotePath string, localPath string) error { + target := fmt.Sprintf("scp download %s@%s:%s", username, host, remotePath) + + t, err := intializeScpSession(ctx, host, port, username, key, "-f", remotePath, target) + if err != nil { + return err + } + defer t.close() + + if err := receiveSCPFile(t.stdout, t.stdin, localPath); err != nil { + return t.fail(err) + } + if err := t.finish(); err != nil { + return err + } + + slog.Info("Downloaded file over SCP", "host", host, "remotePath", remotePath, "localPath", localPath) + return nil +} + +// Uploads a single local file to a remote endpoint +func UploadFileToSCP(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential, localPath string, remotePath string) error { + target := fmt.Sprintf("scp upload %s@%s:%s", username, host, remotePath) + + // Validate the local file before attempting the SCP upload. + file, err := os.Open(localPath) + if err != nil { + return fmt.Errorf("%s: %w", target, err) + } + defer file.Close() + info, err := file.Stat() + if err != nil { + return fmt.Errorf("%s: %w", target, err) + } + if info.IsDir() { + return fmt.Errorf("%s: %s is a directory: recursive scp upload is not supported", target, localPath) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("%s: %s is not a regular file, so the size it would announce cannot be trusted", target, localPath) + } + + t, err := intializeScpSession(ctx, host, port, username, key, "-t", remotePath, target) + if err != nil { + return err + } + defer t.close() + + if err := sendSCPFile(t.stdout, t.stdin, file, filepath.Base(localPath), info.Mode().Perm(), info.Size()); err != nil { + return t.fail(err) + } + if err := t.finish(); err != nil { + return err + } + + slog.Info("Uploaded file over SCP", "host", host, "localPath", localPath, "remotePath", remotePath) + return nil +} + +// Creates remotePath on the host, including any missing parent directories, and does +// nothing if it is already there. +func CreateDirectorySSH(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential, remotePath string) error { + target := fmt.Sprintf("ssh mkdir %s@%s:%s", username, host, remotePath) + + if strings.TrimSpace(remotePath) == "" { + return fmt.Errorf("%s: no remote directory was named", target) + } + + if _, _, err := RunSSHCommand(ctx, host, port, username, key, "mkdir -p "+ShellQuote(remotePath), target); err != nil { + return err + } + + slog.Info("Created directory over SSH", "host", host, "remotePath", remotePath) + return nil +} + +// Reads the mode, size and name a remote scp announces for a single remote file, +// without transferring its contents. +func GetSCPFileMetadata(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential, remotePath string) (*FileMetadata, error) { + target := fmt.Sprintf("scp stat %s@%s:%s", username, host, remotePath) + + t, err := intializeScpSession(ctx, host, port, username, key, "-f", remotePath, target) + if err != nil { + return nil, err + } + defer t.close() + + // The opening acknowledgement invites the header, and nothing beyond it. + if err := scpAck(t.stdin); err != nil { + return nil, t.fail(err) + } + msg, err := readSCPMessage(t.stdout) + if err != nil { + return nil, t.fail(err) + } + if msg[0] != 'C' { + return nil, t.fail(fmt.Errorf("remote path is not a plain file: scp announced %q", msg)) + } + mode, size, name, err := parseSCPFileHeader(msg) + if err != nil { + return nil, t.fail(err) + } + + // Path carries the name scp announced, which is the remote basename rather than + // the path asked about. + return &FileMetadata{ + Path: name, + Size: size, + Mode: mode, + }, nil +} diff --git a/internal/orchestration/ssh.go b/internal/ssh/ssh.go similarity index 73% rename from internal/orchestration/ssh.go rename to internal/ssh/ssh.go index 3fc18e859d..489c782417 100644 --- a/internal/orchestration/ssh.go +++ b/internal/ssh/ssh.go @@ -18,7 +18,7 @@ * under the License. */ -package orchestration +package ssh import ( "bufio" @@ -27,7 +27,6 @@ import ( "errors" "fmt" "io" - "log/slog" "net" "os" "path/filepath" @@ -39,120 +38,113 @@ import ( "golang.org/x/crypto/ssh" ) -// Note: Only use DownloadFileFromSCP and UploadFileToSCP for SCP transfers. Do not use -// any of the utility methods outside of this file. They are considered internal implementation details. Also, -// DO NOT change internal implementation logic unless you fully understand the implications. - -// Copies a single remote file to localPath over SSH -func DownloadFileFromSCP(ctx context.Context, host string, port int, username string, key credmodel.SSHKey, remotePath string, localPath string) error { - target := fmt.Sprintf("scp download %s@%s:%s", username, host, remotePath) +// ========================== Utility section ===================================== - t, err := intializeSession(ctx, host, port, username, key, "-f", remotePath, target) - if err != nil { - return err - } - defer t.close() +// scpOK is the byte each side of the SCP wire protocol writes to acknowledge the +// other's last message. A leading 0x01 or 0x02 instead introduces a one-line error, +// which is the only way a remote scp reports a missing or unreadable path. +const scpOK = 0x00 - if err := receiveSCPFile(t.stdout, t.stdin, localPath); err != nil { - return t.fail(err) - } - if err := t.finish(); err != nil { - return err - } +// scpDialTimeout bounds the TCP connect and the SSH handshake behind it. A staging +// activity is retried by the workflow, so a host that is merely down should fail the +// attempt quickly rather than hold a worker slot until the OS gives up on the socket. +const scpDialTimeout = 30 * time.Second - slog.Info("Downloaded file over SCP", "host", host, "remotePath", remotePath, "localPath", localPath) - return nil +type FileMetadata struct { + Path string + Size int64 + Mode os.FileMode } -// Uploads a single local file to a remote endpoint -func UploadFileToSCP(ctx context.Context, host string, port int, username string, key credmodel.SSHKey, localPath string, remotePath string) error { - target := fmt.Sprintf("scp upload %s@%s:%s", username, host, remotePath) +type scpTransfer struct { + client *ssh.Client + session *ssh.Session + stdin io.WriteCloser + stdout *bufio.Reader + stderr *bytes.Buffer + ctx context.Context + done chan struct{} + target string +} - // Validate the local file before attempting the SCP upload. - file, err := os.Open(localPath) - if err != nil { - return fmt.Errorf("%s: %w", target, err) - } - defer file.Close() - info, err := file.Stat() +func dialSSH(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential) (*ssh.Client, error) { + signer, err := signerFor(key) if err != nil { - return fmt.Errorf("%s: %w", target, err) - } - if info.IsDir() { - return fmt.Errorf("%s: %s is a directory: recursive scp upload is not supported", target, localPath) + return nil, err } - if !info.Mode().IsRegular() { - return fmt.Errorf("%s: %s is not a regular file, so the size it would announce cannot be trusted", target, localPath) + hostKeys := ssh.InsecureIgnoreHostKey() + if port == 0 { + port = 22 } + addr := net.JoinHostPort(host, strconv.Itoa(port)) - t, err := intializeSession(ctx, host, port, username, key, "-t", remotePath, target) + // DialContext rather than ssh.Dial: the activity's context has to be able to + // abandon a connect that is going nowhere. + dialer := net.Dialer{Timeout: scpDialTimeout} + conn, err := dialer.DialContext(ctx, "tcp", addr) if err != nil { - return err - } - defer t.close() - - if err := sendSCPFile(t.stdout, t.stdin, file, filepath.Base(localPath), info.Mode().Perm(), info.Size()); err != nil { - return t.fail(err) - } - if err := t.finish(); err != nil { - return err + return nil, fmt.Errorf("connecting to %s: %w", addr, err) } - - slog.Info("Uploaded file over SCP", "host", host, "localPath", localPath, "remotePath", remotePath) - return nil -} - -// Creates remotePath on the host, including any missing parent directories, and does -// nothing if it is already there. -func CreateDirectorySSH(ctx context.Context, host string, port int, username string, key credmodel.SSHKey, remotePath string) error { - target := fmt.Sprintf("ssh mkdir %s@%s:%s", username, host, remotePath) - - if strings.TrimSpace(remotePath) == "" { - return fmt.Errorf("%s: no remote directory was named", target) + if deadline, ok := ctx.Deadline(); ok { + _ = conn.SetDeadline(deadline) } - - if _, _, err := runSSHCommand(ctx, host, port, username, key, "mkdir -p "+shellQuote(remotePath), target); err != nil { - return err + sshConn, chans, reqs, err := ssh.NewClientConn(conn, addr, &ssh.ClientConfig{ + User: username, + Auth: []ssh.AuthMethod{ssh.PublicKeys(signer)}, + HostKeyCallback: hostKeys, + Timeout: scpDialTimeout, + }) + if err != nil { + conn.Close() + return nil, fmt.Errorf("ssh handshake with %s: %w", addr, err) } - - slog.Info("Created directory over SSH", "host", host, "remotePath", remotePath) - return nil + // The handshake deadline must not outlive the handshake: a transfer reads for as + // long as the file takes. + _ = conn.SetDeadline(time.Time{}) + return ssh.NewClient(sshConn, chans, reqs), nil } -// Reads the mode, size and name a remote scp announces for a single remote file, -// without transferring its contents. -func GetSCPFileMetadata(ctx context.Context, host string, port int, username string, key credmodel.SSHKey, remotePath string) (*FileMetadata, error) { - target := fmt.Sprintf("scp stat %s@%s:%s", username, host, remotePath) - - t, err := intializeSession(ctx, host, port, username, key, "-f", remotePath, target) +func intializeScpSession(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential, direction, remotePath, target string) (*scpTransfer, error) { + client, err := dialSSH(ctx, host, port, username, key) if err != nil { - return nil, err - } - defer t.close() - - // The opening acknowledgement invites the header, and nothing beyond it. - if err := scpAck(t.stdin); err != nil { - return nil, t.fail(err) + return nil, fmt.Errorf("%s: %w", target, err) } - msg, err := readSCPMessage(t.stdout) + session, err := client.NewSession() if err != nil { - return nil, t.fail(err) + client.Close() + return nil, fmt.Errorf("%s: opening session: %w", target, err) } - if msg[0] != 'C' { - return nil, t.fail(fmt.Errorf("remote path is not a plain file: scp announced %q", msg)) + t := &scpTransfer{client: client, session: session, ctx: ctx, target: target, done: make(chan struct{})} + if t.stdin, err = session.StdinPipe(); err != nil { + session.Close() + client.Close() + return nil, fmt.Errorf("%s: %w", target, err) } - mode, size, name, err := parseSCPFileHeader(msg) + stdout, err := session.StdoutPipe() if err != nil { - return nil, t.fail(err) + session.Close() + client.Close() + return nil, fmt.Errorf("%s: %w", target, err) } + t.stdout = bufio.NewReader(stdout) + t.stderr = &bytes.Buffer{} + session.Stderr = t.stderr - // Path carries the name scp announced, which is the remote basename rather than - // the path asked about. - return &FileMetadata{ - Path: name, - Size: size, - Mode: mode, - }, nil + // A session read blocks on the network, so cancellation has to arrive by closing + // the session under it. done stops the watcher when the transfer finishes first. + go func() { + select { + case <-ctx.Done(): + session.Close() + case <-t.done: + } + }() + + if err := session.Start("scp " + direction + " " + ShellQuote(remotePath)); err != nil { + t.close() + return nil, fmt.Errorf("%s: starting remote scp: %w", target, err) + } + return t, nil } // runSSHCommand runs one command on the host and waits for it to finish, returning @@ -164,7 +156,7 @@ func GetSCPFileMetadata(ctx context.Context, host string, port int, username str // to stderr is folded into the error as well, so a caller that only checks err is no // worse off than before. The streams are returned as the command wrote them, trailing // newline included; a caller parsing them should trim. -func runSSHCommand(ctx context.Context, host string, port int, username string, key credmodel.SSHKey, command, target string) (string, string, error) { +func RunSSHCommand(ctx context.Context, host string, port int, username string, key credmodel.SSHCredential, command, target string) (string, string, error) { client, err := dialSSH(ctx, host, port, username, key) if err != nil { return "", "", fmt.Errorf("%s: %w", target, err) @@ -203,72 +195,6 @@ func runSSHCommand(ctx context.Context, host string, port int, username string, return stdout.String(), stderr.String(), nil } -// ========================== Utility section ===================================== - -// scpOK is the byte each side of the SCP wire protocol writes to acknowledge the -// other's last message. A leading 0x01 or 0x02 instead introduces a one-line error, -// which is the only way a remote scp reports a missing or unreadable path. -const scpOK = 0x00 - -// scpDialTimeout bounds the TCP connect and the SSH handshake behind it. A staging -// activity is retried by the workflow, so a host that is merely down should fail the -// attempt quickly rather than hold a worker slot until the OS gives up on the socket. -const scpDialTimeout = 30 * time.Second - -type scpTransfer struct { - client *ssh.Client - session *ssh.Session - stdin io.WriteCloser - stdout *bufio.Reader - stderr *bytes.Buffer - ctx context.Context - done chan struct{} - target string -} - -func intializeSession(ctx context.Context, host string, port int, username string, key credmodel.SSHKey, direction, remotePath, target string) (*scpTransfer, error) { - client, err := dialSSH(ctx, host, port, username, key) - if err != nil { - return nil, fmt.Errorf("%s: %w", target, err) - } - session, err := client.NewSession() - if err != nil { - client.Close() - return nil, fmt.Errorf("%s: opening session: %w", target, err) - } - t := &scpTransfer{client: client, session: session, ctx: ctx, target: target, done: make(chan struct{})} - if t.stdin, err = session.StdinPipe(); err != nil { - session.Close() - client.Close() - return nil, fmt.Errorf("%s: %w", target, err) - } - stdout, err := session.StdoutPipe() - if err != nil { - session.Close() - client.Close() - return nil, fmt.Errorf("%s: %w", target, err) - } - t.stdout = bufio.NewReader(stdout) - t.stderr = &bytes.Buffer{} - session.Stderr = t.stderr - - // A session read blocks on the network, so cancellation has to arrive by closing - // the session under it. done stops the watcher when the transfer finishes first. - go func() { - select { - case <-ctx.Done(): - session.Close() - case <-t.done: - } - }() - - if err := session.Start("scp " + direction + " " + shellQuote(remotePath)); err != nil { - t.close() - return nil, fmt.Errorf("%s: starting remote scp: %w", target, err) - } - return t, nil -} - func (t *scpTransfer) close() { close(t.done) t.session.Close() @@ -490,61 +416,24 @@ func scpAck(w io.Writer) error { return nil } -func dialSSH(ctx context.Context, host string, port int, username string, key credmodel.SSHKey) (*ssh.Client, error) { - signer, err := signerFor(key) - if err != nil { - return nil, err - } - hostKeys := ssh.InsecureIgnoreHostKey() - if port == 0 { - port = 22 - } - addr := net.JoinHostPort(host, strconv.Itoa(port)) - - // DialContext rather than ssh.Dial: the activity's context has to be able to - // abandon a connect that is going nowhere. - dialer := net.Dialer{Timeout: scpDialTimeout} - conn, err := dialer.DialContext(ctx, "tcp", addr) - if err != nil { - return nil, fmt.Errorf("connecting to %s: %w", addr, err) - } - if deadline, ok := ctx.Deadline(); ok { - _ = conn.SetDeadline(deadline) - } - sshConn, chans, reqs, err := ssh.NewClientConn(conn, addr, &ssh.ClientConfig{ - User: username, - Auth: []ssh.AuthMethod{ssh.PublicKeys(signer)}, - HostKeyCallback: hostKeys, - Timeout: scpDialTimeout, - }) - if err != nil { - conn.Close() - return nil, fmt.Errorf("ssh handshake with %s: %w", addr, err) - } - // The handshake deadline must not outlive the handshake: a transfer reads for as - // long as the file takes. - _ = conn.SetDeadline(time.Time{}) - return ssh.NewClient(sshConn, chans, reqs), nil -} - -func signerFor(key credmodel.SSHKey) (ssh.Signer, error) { +func signerFor(key credmodel.SSHCredential) (ssh.Signer, error) { pem := []byte(key.PrivateKey) if key.Passphrase != nil && *key.Passphrase != "" { signer, err := ssh.ParsePrivateKeyWithPassphrase(pem, []byte(*key.Passphrase)) if err != nil { - return nil, fmt.Errorf("ssh key %s could not be decrypted with its registered passphrase", key.ID) + return nil, fmt.Errorf("ssh credential %s could not be decrypted with its registered passphrase", key.ID) } return signer, nil } signer, err := ssh.ParsePrivateKey(pem) if err != nil { - return nil, fmt.Errorf("ssh key %s could not be parsed (is it passphrase protected without a registered passphrase?)", key.ID) + return nil, fmt.Errorf("ssh credential %s could not be parsed (is it passphrase protected without a registered passphrase?)", key.ID) } return signer, nil } -// shellQuote wraps s for the remote login shell, which is what runs the scp command +// ShellQuote wraps s for the remote login shell, which is what runs the scp command // line. A path is user supplied, so it cannot be interpolated bare. -func shellQuote(s string) string { +func ShellQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } diff --git a/internal/orchestration/ssh_test.go b/internal/ssh/ssh_test.go similarity index 98% rename from internal/orchestration/ssh_test.go rename to internal/ssh/ssh_test.go index 867699e0ba..760351294d 100644 --- a/internal/orchestration/ssh_test.go +++ b/internal/ssh/ssh_test.go @@ -18,7 +18,7 @@ * under the License. */ -package orchestration +package ssh import ( "os" @@ -55,7 +55,7 @@ func TestSCPDownload(t *testing.T) { startTime := time.Now() - err = DownloadFileFromSCP(t.Context(), host, 22, username, credmodel.SSHKey{ + err = DownloadFileFromSCP(t.Context(), host, 22, username, credmodel.SSHCredential{ PrivateKey: privateKeyStr, Passphrase: &passphrase, }, "/home/"+username+"/random.bin", "/tmp/random.bin") @@ -104,7 +104,7 @@ func TestSCPUpload(t *testing.T) { startTime := time.Now() - err = UploadFileToSCP(t.Context(), host, 22, username, credmodel.SSHKey{ + err = UploadFileToSCP(t.Context(), host, 22, username, credmodel.SSHCredential{ PrivateKey: privateKeyStr, Passphrase: &passphrase, }, "/tmp/random.bin", "/home/"+username+"/randomupload.bin") @@ -113,7 +113,7 @@ func TestSCPUpload(t *testing.T) { } t.Logf("SCP upload completed in %v", time.Since(startTime)) - metadata, err := GetSCPFileMetadata(t.Context(), host, 22, username, credmodel.SSHKey{ + metadata, err := GetSCPFileMetadata(t.Context(), host, 22, username, credmodel.SSHCredential{ PrivateKey: privateKeyStr, Passphrase: &passphrase, }, "/home/"+username+"/randomupload.bin")