From dcd2b6671ba70cda445a6dedc501a2f89ca9a52a Mon Sep 17 00:00:00 2001 From: Fergal Mc Carthy Date: Wed, 5 Feb 2025 14:57:45 -0500 Subject: [PATCH 1/5] Rework client registration and clientId management Previously the clientId was the Id assigned by the upstream telemetry service when a client was registered. This has been renamed to the registrationId, along with the accompanying request header and the associated fields in the registration response and re-authentication request. Previously a client system would generated a client instance id value and store it in a file; this has now been renamed to a registration value. Where previously the client instance id was just a single string value derived from the timestamp when the value was generated, it now consists of three values, namely a clientId as a UUID string, an optional systemUUID (if available), and a UTC timestamp for when the registration was generated. Where previously this registrationId from the client credentials was used as the clientId field in telemetry bundle and report structures, we now will use the clientId value from the client's registration. Updated docs to reflect these changes. --- doc/api/README.md | 16 +- ...ent-id.md => telemetry-registration-id.md} | 6 +- doc/api/requests/authenticate.md | 8 +- doc/api/requests/register.md | 8 +- doc/api/structs/telemetrybundle.md | 6 +- doc/api/structs/telemetryreport.md | 6 +- pkg/client/authenticate.go | 10 +- pkg/client/client.go | 112 +++++++------ pkg/client/register.go | 14 +- pkg/client/registration_management.go | 156 ++++++++++++++++++ pkg/client/report.go | 2 +- pkg/client/systemuuid.go | 41 +++++ pkg/lib/bundles.go | 10 +- pkg/lib/processor.go | 8 +- pkg/lib/processor_test.go | 8 +- pkg/lib/reports.go | 10 +- pkg/restapi/restapi.go | 8 +- pkg/types/types.go | 24 ++- telemetry.go | 12 +- 19 files changed, 344 insertions(+), 121 deletions(-) rename doc/api/headers/{telemetry-client-id.md => telemetry-registration-id.md} (58%) create mode 100644 pkg/client/registration_management.go create mode 100644 pkg/client/systemuuid.go diff --git a/doc/api/README.md b/doc/api/README.md index fcdaaf8..f60d3d1 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -15,20 +15,20 @@ A telemetry client is expected to support the following workflow: # Registration For a telemetry client to be able to register with an upstream telemetry -server, it will need to generate a clientInstanceId value, which is used -to uniquely identify a given client with the upstream server, and should +server, it will need to generate a registration value, which is used to +uniquely identify a given client system with the upstream server, and should store this value in a secure fashion so that it can be accessed later when (re-)authenticating with the upstream telemetry server. When a telemetry client registers with the upstream telemetry server, using the [/register](requests/register.md) request, it will send a request payload -containing the clientInstanceId, and the successful response will provide +containing the registration, and the successful response will provide a set of client credentials as follows: | Name | Type | Description | | ---- | ---- | ----------- | -| clientId | integer($int64) | ID used to identify the client to the server | -| authToken | string($([JWT](https://jwt.io/)) | A JSON Web Token ([JWT](https://jwt.io/)) authorization token | +| registrationId | integer($int64) | ID used to identify the client system to the server | +| authToken | string($([jwt](https://jwt.io/)) | A JSON Web Token ([JWT](https://jwt.io/)) authorization token | | registrationDate | string($[rfc3339nano](https://pkg.go.dev/time#pkg-constants)) | The client UTC registration timestamp expressed in
[RFC3339nano](https://pkg.go.dev/time#pkg-constants) format | ***NOTE***: The telemetry client is responsible for storing these client @@ -41,7 +41,7 @@ server it must prove that it has the authorization to do so. This is achieved by supplying the appropriate request headers: * [Authorization](headers/authorization.md) -* [X-Telemetry-Client-Id](headers/telemetry-client-id.md) +* [X-Telemetry-Registration-Id](headers/telemetry-registration-id.md) When a telemetry client submits a telemetry report to the upstream telemetry server, using the [/report](requests/report.md) @@ -54,11 +54,11 @@ objects. # (Re-)Authentication For a telemetry client to (re-)authenticate with an upstream telemetry server, it will need to generate a supported hash, e.g. `sha256`, of the -clientInstanceId to validate that it is in fact that client in question. +registration to validate that it is in fact that client in question. When a telemetry client (re-)authenticates with the upstream telemetry server, using the [/authenticate](requests/authenticate.md) request, it will -send a request payload containing it's clientId and an instIdHash, +send a request payload containing it's registratiionId and an registrationHash, specifying the hash method and associated value, and the successful response will provide a set of client credentials, the same as for a [/register](requests/register.md) request, as described [above][#registration]. \ No newline at end of file diff --git a/doc/api/headers/telemetry-client-id.md b/doc/api/headers/telemetry-registration-id.md similarity index 58% rename from doc/api/headers/telemetry-client-id.md rename to doc/api/headers/telemetry-registration-id.md index 387da5d..1874e3d 100644 --- a/doc/api/headers/telemetry-client-id.md +++ b/doc/api/headers/telemetry-registration-id.md @@ -1,9 +1,9 @@ # Telemetry Client Id Header When a telemetry client is submitting a telemetry report using the [/report](../requests/report.md) request it will need to provide a -`X-Telemetry-Client-Id` header specifying the clientId from the client +`X-Telemetry-Registration-Id` header specifying the registrationId from the client credentials obtained using the [/register](../requests/register.md) request. ## Format of the Telemetry Client Id Header -The `X-Telemetry-Client-Id` header value should be the string -representation of the int64 clientId value. \ No newline at end of file +The `X-Telemetry-Registration-Id` header value should be the string +representation of the int64 registrationId value. \ No newline at end of file diff --git a/doc/api/requests/authenticate.md b/doc/api/requests/authenticate.md index dd04af8..a913c3a 100644 --- a/doc/api/requests/authenticate.md +++ b/doc/api/requests/authenticate.md @@ -6,7 +6,7 @@ Type: **POST** | Name | Type | Description | Example | | ---- | ---- | ----------- | ------- | -| body | object | {
  clientId integer($int64)
  instIdHash {
    method string
    value string
  }
} | {
  "clientId": 1234567890
  "instIdHash": {
    "method": "sha256"
    "value": "984271ec70628b47995fdf9271ded6274c2b104ce201164a9b63cfefef7f40d0"
  }
}| +| body | object | {
  registrationId integer($int64)
  regHash {
    method string
    value string
  }
} | {
  "registrationId": 1234567890
  "regHash": {
    "method": "sha256"
    "value": "984271ec70628b47995fdf9271ded6274c2b104ce201164a9b63cfefef7f40d0"
  }
}| Request body type `ClientAuthenticationRequest` defined in [restapi module](../../../pkg/restapi) @@ -14,9 +14,9 @@ Request body type `ClientAuthenticationRequest` defined in [restapi module](../. | Code | Description | Example | | ---- | ----------- | ------- | -| 200 | Success
`Content-Type: application/json`
{
  clientId integer($int64)
  authToken string
  registrationDate string
} | {
  "clientId": 1234567890
  "authToken": "encoded.JWT.token"
  "registrationDate": "2024-08-01T01:02:03.000000Z"
} | -| 400 | Bad Request
Missing or incompatible body
`Content-Type: application/json`
{
  error string
} | {
  "error": "no clientInstanceId value provided"
} | -| 401 | Unauthorized
Client (re-)registration required due to one of:
- specified client is not registered
- invalid clientId provided
- provided clientInstanceId hash doesn't match
[WWW-Authenticate](../headers/www-authenticate.md) response header will specify recovery action
`Content-Type: application/json`
{
  error string
} | {
  "error": "Client not registered"
} | +| 200 | Success
`Content-Type: application/json`
{
  registrationId integer($int64)
  authToken string
  registrationDate string
} | {
  "registrationId": 1234567890
  "authToken": "encoded.JWT.token"
  "registrationDate": "2024-08-01T01:02:03.000000Z"
} | +| 400 | Bad Request
Missing or incompatible body
`Content-Type: application/json`
{
  error string
} | {
  "error": "no registration hash value provided"
} | +| 401 | Unauthorized
Client (re-)registration required due to one of:
- specified client is not registered
- invalid registrationId provided
- provided registration hash doesn't match
[WWW-Authenticate](../headers/www-authenticate.md) response header will specify recovery action
`Content-Type: application/json`
{
  error string
} | {
  "error": "Client not registered"
} | Response success body type `ClientAuthenticationResponse` defined in [restapi module](../../../pkg/restapi) diff --git a/doc/api/requests/register.md b/doc/api/requests/register.md index 1897707..87a0934 100644 --- a/doc/api/requests/register.md +++ b/doc/api/requests/register.md @@ -6,7 +6,7 @@ Type: **POST** | Name | Type | Description | Example | | ---- | ---- | ----------- | ------- | -| body | object | {
  clientInstanceId: string
} | {
  "clientInstanceId": "ba2cb9f4927441602a385b27f502134902b636f395cadb3ea1438084dba29c8c"
}| +| body | object | {
  clientRegistration: {
    clientId: string
    systemUUID: string
    timestamp: string($[rfc3339nano](https://pkg.go.dev/time#pkg-constants))
  }
} | {
  "clientRegistration": {
    "clientId": "f323628e-c1cc-45d4-824d-22d4d6f0fd01"
    "systemUUID": "74f0f0b0-fb29-4405-a0b8-4e7747bdfd8a"
    "timestamp": "2024-08-01T00:01:02.000000Z"
  }
}| Request body type `ClientRegistrationRequest` defined in [restapi module](../../../pkg/restapi/) @@ -14,8 +14,8 @@ Request body type `ClientRegistrationRequest` defined in [restapi module](../../ | Code | Description | Example | | ---- | ----------- | ------- | -| 200 | Success
`Content-Type: application/json`
{
  clientId integer($int64)
  authToken string
  registrationDate string
} | {
  "clientId": 1234567890
  "authToken": "encoded.JWT.token"
  "registrationDate": "2024-08-01T01:02:03.000000Z"
} | -| 400 | Bad Request
Missing or incompatible body
`Content-Type: application/json`
{
  error string
} | {
  "error": "no clientInstanceId value provided"
} | -| 409 | Conflict
Client Instance Id already registered
`Content-Type: application/json`
{
  error string
} | {
  "error": "specified clientInstanceId already exists"
} | +| 200 | Success
`Content-Type: application/json`
{
  registrationId integer($int64)
  authToken string
  registrationDate string
} | {
  "registrationId": 1234567890
  "authToken": "encoded.JWT.token"
  "registrationDate": "2024-08-01T01:02:03.000000Z"
} | +| 400 | Bad Request
Missing or incompatible body
`Content-Type: application/json`
{
  error string
} | {
  "error": "missing registration clientId"
}
or
{
  "error": "missing registration timespace"
} | +| 409 | Conflict
Client Registration already registered
`Content-Type: application/json`
{
  error string
} | {
  "error": "specified registration already exists"
} | Response success body type `ClientRegistrationResponse` defined in [restapi module](../../../pkg/restapi/) \ No newline at end of file diff --git a/doc/api/structs/telemetrybundle.md b/doc/api/structs/telemetrybundle.md index 20afa6a..c8884be 100644 --- a/doc/api/structs/telemetrybundle.md +++ b/doc/api/structs/telemetrybundle.md @@ -8,8 +8,8 @@ The TelemetryBundle data structure consists of the following sections: * bundleTimeStamp - the UTC timestamp for when the bundle was generated, formatted in [RFC3339nano](../../telemetrytimestamp.md) format * bundleClientId - the clientId of the telemetry client generating this - bundle, as specified in the credentials returned when the telemetry - client successfully registered with the upstream telemetry server. + bundle, as specified in the registraion used to successfully register + the client with the upstream telemetry server. * bundleCustomerId - a string value specify the customer identifier, if any, associated with the telemetry client * bundleAnnotations - a possibly empty list of @@ -25,7 +25,7 @@ in a bundle must originate from the same telemetry client. header { bundleId string bundleTimeStamp string($rfc3339nano) - bundleClientId integer($int64) + bundleClientId string bundleCustomerId string bundleAnnotations [ string... diff --git a/doc/api/structs/telemetryreport.md b/doc/api/structs/telemetryreport.md index cbb71fc..3fa708d 100644 --- a/doc/api/structs/telemetryreport.md +++ b/doc/api/structs/telemetryreport.md @@ -8,8 +8,8 @@ The TelemetryReport data structure consists of the following sections: * reportTimeStamp - the UTC timestamp for when the bundle was generated, formatted in [RFC3339nano](../../telemetrytimestamp.md) format * reportClientId - the clientId of the telemetry client generating this - report, as specified in the credentials returned when the telemetry - client successfully registered with the upstream telemetry server. + report, as specified in the registraion used to successfully register + the client with the upstream telemetry server. * reportAnnotations - a possibly empty list of [telemetry annotation tags](../../telemetrytag.md) * payload - a list of one or more [TelemetryBundle](telemetrybundle.md) objects @@ -24,7 +24,7 @@ relayed through one or more telemetry relays. header { reportId string reportTimeStamp string($rfc3339nano) - reportClientId integer($int64) + reportClientId string reportAnnotations [ string... ] diff --git a/pkg/client/authenticate.go b/pkg/client/authenticate.go index 47a2d88..e10265a 100644 --- a/pkg/client/authenticate.go +++ b/pkg/client/authenticate.go @@ -24,15 +24,15 @@ func (tc *TelemetryClient) Authenticate() (err error) { } // get the instanceId, failing if it can't be retrieved - instId, err := tc.getInstanceId() + regId, err := tc.getRegistration() if err != nil { return } // assemble the authentication request caReq := restapi.ClientAuthenticationRequest{ - ClientId: tc.auth.ClientId, - InstIdHash: *instId.Hash("default"), + RegistrationId: tc.auth.RegistrationId, + RegHash: *regId.Hash("default"), } reqBodyJSON, err := json.Marshal(&caReq) @@ -92,7 +92,7 @@ func (tc *TelemetryClient) Authenticate() (err error) { return } - tc.auth.ClientId = caResp.ClientId + tc.auth.RegistrationId = caResp.RegistrationId tc.auth.Token = types.TelemetryAuthToken(caResp.AuthToken) tc.auth.RegistrationDate, err = types.TimeStampFromString(caResp.RegistrationDate) if err != nil { @@ -115,7 +115,7 @@ func (tc *TelemetryClient) Authenticate() (err error) { slog.Debug( "successfully authenticated", - slog.Int64("clientId", tc.auth.ClientId), + slog.Int64("registrationId", tc.auth.RegistrationId), ) return diff --git a/pkg/client/client.go b/pkg/client/client.go index 5f20017..72fdb41 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -1,7 +1,6 @@ package client import ( - "encoding/base64" "encoding/json" "errors" "fmt" @@ -20,20 +19,23 @@ import ( const ( //CONFIG_DIR = "/etc/susetelemetry" - CONFIG_DIR = "/tmp/susetelemetry" - CONFIG_PATH = CONFIG_DIR + "/config.yaml" - AUTH_PATH = CONFIG_DIR + "/auth.json" - INSTANCEID_PATH = CONFIG_DIR + "/instanceid" + CONFIG_DIR = "/tmp/susetelemetry" + CONFIG_PATH = CONFIG_DIR + "/config.yaml" + AUTH_PATH = CONFIG_DIR + "/auth.json" + //INSTANCEID_PATH = CONFIG_DIR + "/instanceid" ) +// TODO: unify with restapi.ClientRegistrationResponse type TelemetryAuth struct { - ClientId int64 `json:"clientId"` - Token types.TelemetryAuthToken `json:"token"` + // TODO: unify these fields with restapi.ClientRegistrationResponse + RegistrationId int64 `json:"registrationId"` + Token types.TelemetryAuthToken `json:"authToken"` RegistrationDate types.TelemetryTimeStamp `json:"registrationDate"` } type TelemetryClient struct { cfg *config.Config + reg *TelemetryClientRegistration auth TelemetryAuth authLoaded bool processor telemetrylib.TelemetryProcessor @@ -41,6 +43,7 @@ type TelemetryClient struct { func NewTelemetryClient(cfg *config.Config) (tc *TelemetryClient, err error) { tc = &TelemetryClient{cfg: cfg} + tc.reg = NewTelemetryClientRegistration() tc.processor, err = telemetrylib.NewTelemetryProcessor(&cfg.DataStores) return } @@ -69,30 +72,46 @@ func checkFileReadAccessible(filePath string) bool { return true } -func ensureInstanceIdExists(instIdPath string) error { +func (tc *TelemetryClient) ensureRegistrationExists() (err error) { - slog.Debug("ensuring existence of instIdPath", slog.String("instIdPath", instIdPath)) - _, err := os.Stat(instIdPath) - if !os.IsNotExist(err) { + // if the client registration is valid and accessible then nothing to do + if tc.reg.Valid() && tc.reg.Accessible() { + slog.Debug( + "client registration exists", + slog.String("reg", tc.reg.String()), + ) return nil } - // For now generate an instanceId as a base64 encoded timestamp - now := types.Now().String() - instId := make([]byte, base64.StdEncoding.EncodedLen(len(now))) - base64.StdEncoding.Encode(instId, []byte(now)) + slog.Debug( + "ensuring existence of client registration", + slog.String("regPath", tc.reg.path), + ) + + // generate a new client registration if needed + if !tc.reg.Valid() { + tc.reg.Generate() + slog.Debug( + "client registration generated", + slog.String("reg", tc.reg.String()), + ) + } - err = os.WriteFile(instIdPath, instId, 0600) + err = tc.reg.Save() if err != nil { - slog.Error( - "failed to write instId to instIdPath", - slog.String("instId", string(instId)), - slog.String("instIdPath", instIdPath), - slog.String("err", err.Error()), + slog.Debug( + "failed to save client registration generated", + slog.String("reg", tc.reg.String()), ) + return err } - return nil + slog.Debug( + "saved client registration", + slog.String("reg", tc.reg.String()), + ) + + return } func (tc *TelemetryClient) authParsedToken() (token *jwt.Token, err error) { @@ -164,16 +183,12 @@ func (tc *TelemetryClient) AuthAccessible() bool { return checkFileReadAccessible(tc.AuthPath()) } -func (tc *TelemetryClient) InstanceIdAccessible() bool { - return checkFileReadAccessible(tc.InstIdPath()) -} - func (tc *TelemetryClient) HasAuth() bool { return checkFileExists(tc.AuthPath()) } func (tc *TelemetryClient) HasInstanceId() bool { - return checkFileExists(tc.InstIdPath()) + return checkFileExists(tc.RegistrationPath()) } func (tc *TelemetryClient) Processor() telemetrylib.TelemetryProcessor { @@ -186,37 +201,42 @@ func (tc *TelemetryClient) AuthPath() string { return AUTH_PATH } -func (tc *TelemetryClient) InstIdPath() string { - // hard coded for now, possibly make a config option - return INSTANCEID_PATH +func (tc *TelemetryClient) ClientId() string { + return tc.reg.ClientId +} + +func (tc *TelemetryClient) RegistrationPath() string { + return tc.reg.path +} + +func (tc *TelemetryClient) RegistrationAccessible() bool { + return tc.reg.Accessible() } -func (tc *TelemetryClient) getInstanceId() (instId types.ClientInstanceId, err error) { - instIdPath := tc.InstIdPath() +func (tc *TelemetryClient) getRegistration() (reg types.ClientRegistration, err error) { - err = ensureInstanceIdExists(instIdPath) + err = tc.ensureRegistrationExists() if err != nil { return } - data, err := os.ReadFile(instIdPath) + err = tc.reg.Load() if err != nil { - slog.Error( - "failed to read instId file", - slog.String("path", instIdPath), + slog.Debug( + "failed to load client registration", + slog.String("reg", tc.reg.Path()), slog.String("err", err.Error()), ) return } - instId = types.ClientInstanceId((data)) - slog.Debug( - "successfully read instId file", - slog.String("path", string(instIdPath)), - slog.String("instId", string(instId)), + "successfully loaded client registration", + slog.String("reg", tc.reg.String()), ) + reg = tc.reg.ClientRegistration + return } @@ -271,8 +291,8 @@ func (tc *TelemetryClient) loadTelemetryAuth() (err error) { return } - if tc.auth.ClientId <= 0 { - err = fmt.Errorf("invalid client id") + if tc.auth.RegistrationId <= 0 { + err = fmt.Errorf("invalid registration id") slog.Error( "invalid auth", slog.String("authPath", authPath), @@ -494,7 +514,7 @@ func (tc *TelemetryClient) Generate(telemetry types.TelemetryType, content *type func (tc *TelemetryClient) CreateBundles(tags types.Tags) error { // Bundle existing telemetry data items found in DataItem data store into one or more bundles in the Bundle data store slog.Debug("Bundle", slog.String("Tags", tags.String())) - tc.processor.GenerateBundle(tc.auth.ClientId, tc.cfg.CustomerID, tags) + tc.processor.GenerateBundle(tc.ClientId(), tc.cfg.CustomerID, tags) return nil } @@ -502,7 +522,7 @@ func (tc *TelemetryClient) CreateBundles(tags types.Tags) error { func (tc *TelemetryClient) CreateReports(tags types.Tags) (err error) { // Generate reports from available bundles slog.Debug("CreateReports", slog.String("Tags", tags.String())) - tc.processor.GenerateReport(tc.auth.ClientId, tags) + tc.processor.GenerateReport(tc.ClientId(), tags) return } diff --git a/pkg/client/register.go b/pkg/client/register.go index 2f20087..fa5c45c 100644 --- a/pkg/client/register.go +++ b/pkg/client/register.go @@ -16,19 +16,19 @@ func (tc *TelemetryClient) Register() (err error) { // get the saved TelemetryAuth, returning success if found err = tc.loadTelemetryAuth() if err == nil { - slog.Debug("telemetry auth found, client already registered, skipping", slog.Int64("clientId", tc.auth.ClientId)) + slog.Debug("telemetry auth found, client already registered, skipping", slog.Int64("registrationId", tc.auth.RegistrationId)) return } - // get the instanceId, failing if it can't be retrieved - instId, err := tc.getInstanceId() + // get the registration, failing if it can't be retrieved + reg, err := tc.getRegistration() if err != nil { return } // register the system as a client var crReq restapi.ClientRegistrationRequest - crReq.ClientInstanceId = instId + crReq.ClientRegistration = reg reqBodyJSON, err := json.Marshal(&crReq) if err != nil { slog.Error( @@ -71,7 +71,7 @@ func (tc *TelemetryClient) Register() (err error) { return } - // TODO: Handle http.StatusConflict (409) as needing to regenerate instId + // TODO: Handle http.StatusConflict (409) as needing to regenerate registration if resp.StatusCode != http.StatusOK { err = fmt.Errorf("client registration failed: %s", string(respBody)) return @@ -87,7 +87,7 @@ func (tc *TelemetryClient) Register() (err error) { return } - tc.auth.ClientId = crResp.ClientId + tc.auth.RegistrationId = crResp.RegistrationId tc.auth.Token = types.TelemetryAuthToken(crResp.AuthToken) tc.auth.RegistrationDate, err = types.TimeStampFromString(crResp.RegistrationDate) if err != nil { @@ -112,7 +112,7 @@ func (tc *TelemetryClient) Register() (err error) { slog.Debug( "successfully registered as client", - slog.Int64("clientId", tc.auth.ClientId), + slog.Int64("registrationId", tc.auth.RegistrationId), ) return nil diff --git a/pkg/client/registration_management.go b/pkg/client/registration_management.go new file mode 100644 index 0000000..16f9a4e --- /dev/null +++ b/pkg/client/registration_management.go @@ -0,0 +1,156 @@ +package client + +import ( + "encoding/json" + "errors" + "fmt" + "io/fs" + "log/slog" + "os" + + "github.com/SUSE/telemetry/pkg/types" + "github.com/google/uuid" +) + +const ( + REGISTRATION_PATH = CONFIG_DIR + "/registration" +) + +type TelemetryClientRegistration struct { + types.ClientRegistration + path string + valid bool +} + +func NewTelemetryClientRegistration() *TelemetryClientRegistration { + return &TelemetryClientRegistration{ + path: REGISTRATION_PATH, + valid: false, + } +} + +func (r *TelemetryClientRegistration) Valid() bool { + return r.valid +} + +func (r *TelemetryClientRegistration) Path() string { + return r.path +} + +// for testing purposes +func (r *TelemetryClientRegistration) SetPath(path string) { + r.path = path +} + +func (r *TelemetryClientRegistration) Accessible() bool { + if _, err := os.Open(r.path); err != nil { + return false + } + return true +} + +func (r *TelemetryClientRegistration) Generate() { + r.ClientId = uuid.New().String() + r.SystemUUID = getSystemUUID() + r.Timestamp = types.Now().String() + r.valid = true +} + +func (r *TelemetryClientRegistration) Registration() types.ClientRegistration { + return r.ClientRegistration +} + +func (r *TelemetryClientRegistration) String() string { + return fmt.Sprintf( + "", + r.path, + r.valid, + r.ClientRegistration.String(), + ) +} + +func (r *TelemetryClientRegistration) Save() (err error) { + // saving an invalid registration is not supported + if !r.valid { + return fmt.Errorf("client registration not valid; cannot save") + } + + // marshal the registration public fields as JSON + bytes, err := json.Marshal(r) + if err != nil { + slog.Error( + "failed to json.Marshal() client registration", + slog.String("regId", r.String()), + slog.String("err", err.Error()), + ) + return + } + + // save the JSON encoded registration fields + err = os.WriteFile(r.path, bytes, 0600) + if err != nil { + slog.Error( + "failed to write client registration file", + slog.String("regId", r.String()), + slog.String("err", err.Error()), + ) + return + } + + slog.Debug( + "client registration saved", + slog.String("regId", r.String()), + ) + return +} + +func (r *TelemetryClientRegistration) Load() (err error) { + // check that the specified path exists, failing appropriately + // if there are any issues os.Stat()ing the file. + _, err = os.Stat(r.path) + if err != nil { + var msg string + if errors.Is(err, fs.ErrNotExist) { + msg = "client registration file not found" + } else { + msg = "unable to os.Stat() client registration file" + } + slog.Error( + msg, + slog.String("regIdPath", r.path), + slog.String("err", err.Error()), + ) + return + } + + // retrieve the contents of the specified client registration file, + // failing if unable to do so. + bytes, err := os.ReadFile(r.path) + if err != nil { + slog.Error( + "failed to read client registration file", + slog.String("regIdPath", r.path), + slog.String("err", err.Error()), + ) + return + } + + // unmarshal the contents of the client registration file into the + // client registration structure + err = json.Unmarshal(bytes, r) + if err != nil { + slog.Error( + "failed to json.Unmarshal() client registration file contents", + slog.String("regIdPath", r.path), + slog.String("contents", string(bytes)), + slog.String("err", err.Error()), + ) + return + } + + slog.Debug( + "client registration loaded", + slog.String("regId", r.String()), + ) + return +} diff --git a/pkg/client/report.go b/pkg/client/report.go index fa03b30..0a447bc 100644 --- a/pkg/client/report.go +++ b/pkg/client/report.go @@ -34,7 +34,7 @@ func (tc *TelemetryClient) submitReportInternal(report *telemetrylib.TelemetryRe req.Header.Add("Content-Type", "application/json") req.Header.Add("Authorization", "Bearer "+tc.auth.Token.String()) - req.Header.Add("X-Telemetry-Client-Id", fmt.Sprintf("%d", tc.auth.ClientId)) + req.Header.Add("X-Telemetry-Registration-Id", fmt.Sprintf("%d", tc.auth.RegistrationId)) httpClient := http.DefaultClient resp, err := httpClient.Do(req) diff --git a/pkg/client/systemuuid.go b/pkg/client/systemuuid.go new file mode 100644 index 0000000..1a9cc3d --- /dev/null +++ b/pkg/client/systemuuid.go @@ -0,0 +1,41 @@ +package client + +import ( + "log/slog" + "os" +) + +const ( + // Path to Linux hardware UUID file under /sys + LINUX_SYSTEM_UUID_PATH = "/sys/class/dmi/id/product_uuid" +) + +// retrieve the system uuid +func getSystemUUID() string { + // TODO: determine appropriate environment specific system UUID path + sysuuidPath := LINUX_SYSTEM_UUID_PATH + + // if identified system UUID path doesn't exist, return empty string + if !checkFileExists(sysuuidPath) { + slog.Debug( + "Unable to locate the Linux hardware UUID", + slog.String("path", sysuuidPath), + ) + return "" + } + + // if identified system UUID path can't be read, return empty string + // NOTE: retrieving the contents may require superuser privileges. + uuid, err := os.ReadFile(sysuuidPath) + if err != nil { + slog.Debug( + "unable to retrieve the system UUID - superuser privs may be required", + slog.String("path", sysuuidPath), + slog.String("err", err.Error()), + ) + return "" + } + + // return the retrieved system uuid + return string(uuid) +} diff --git a/pkg/lib/bundles.go b/pkg/lib/bundles.go index e830823..463cc3c 100644 --- a/pkg/lib/bundles.go +++ b/pkg/lib/bundles.go @@ -15,7 +15,7 @@ type TelemetryBundle struct { Footer TelemetryBundleFooter `json:"footer" validate:"required"` } -func NewTelemetryBundle(clientId int64, customerId string, tags types.Tags) *TelemetryBundle { +func NewTelemetryBundle(clientId string, customerId string, tags types.Tags) *TelemetryBundle { tb := new(TelemetryBundle) // fill in header fields @@ -36,7 +36,7 @@ func NewTelemetryBundle(clientId int64, customerId string, tags types.Tags) *Tel type TelemetryBundleHeader struct { BundleId string `json:"bundleId" validate:"required"` BundleTimeStamp string `json:"bundleTimeStamp" validate:"required"` - BundleClientId int64 `json:"bundleClientId" validate:"required"` + BundleClientId string `json:"bundleClientId" validate:"required"` BundleCustomerId string `json:"buncleCustomerId" validate:"required"` BundleAnnotations []string `json:"bundleAnnotations"` } @@ -51,7 +51,7 @@ const bundlesColumns = `( id INTEGER NOT NULL PRIMARY KEY, bundleId VARCHAR(64) NOT NULL, bundleTimestamp VARCHAR(32) NOT NULL, - bundleClientId INTEGER NOT NULL, + bundleClientId VARCHAR NOT NULL, bundleCustomerId VARCHAR(64) NOT NULL, bundleAnnotations TEXT, bundleChecksum VARCHAR(256), @@ -66,14 +66,14 @@ type TelemetryBundleRow struct { Id int64 BundleId string BundleTimestamp string - BundleClientId int64 + BundleClientId string BundleCustomerId string BundleAnnotations string BundleChecksum string ReportId sql.NullInt64 } -func NewTelemetryBundleRow(clientId int64, customerId string, tags types.Tags) (*TelemetryBundleRow, error) { +func NewTelemetryBundleRow(clientId string, customerId string, tags types.Tags) (*TelemetryBundleRow, error) { bundle := NewTelemetryBundle(clientId, customerId, tags) bundleRow := new(TelemetryBundleRow) diff --git a/pkg/lib/processor.go b/pkg/lib/processor.go index 2d8b070..0dae1e7 100644 --- a/pkg/lib/processor.go +++ b/pkg/lib/processor.go @@ -21,14 +21,14 @@ type TelemetryProcessor interface { // Generate telemetry bundle GenerateBundle( - clientId int64, + clientId string, customerId string, tags types.Tags, ) (bundleRow *TelemetryBundleRow, err error) // Generate telemetry report GenerateReport( - clientId int64, + clientId string, tags types.Tags, ) (reportRow *TelemetryReportRow, err error) @@ -111,7 +111,7 @@ func (p *TelemetryProcessorImpl) AddData(telemetry types.TelemetryType, marshale return dataItemRow.Insert(p.t.storer.Conn) } -func (p *TelemetryProcessorImpl) GenerateBundle(clientId int64, customerId string, tags types.Tags) (bundleRow *TelemetryBundleRow, err error) { +func (p *TelemetryProcessorImpl) GenerateBundle(clientId string, customerId string, tags types.Tags) (bundleRow *TelemetryBundleRow, err error) { bundleRow, err = NewTelemetryBundleRow(clientId, customerId, tags) if err != nil { @@ -132,7 +132,7 @@ func (p *TelemetryProcessorImpl) GenerateBundle(clientId int64, customerId strin return } -func (p *TelemetryProcessorImpl) GenerateReport(clientId int64, tags types.Tags) (reportRow *TelemetryReportRow, err error) { +func (p *TelemetryProcessorImpl) GenerateReport(clientId string, tags types.Tags) (reportRow *TelemetryReportRow, err error) { reportRow, err = NewTelemetryReportRow(clientId, tags) if err != nil { diff --git a/pkg/lib/processor_test.go b/pkg/lib/processor_test.go index 6b26fd8..f29f871 100644 --- a/pkg/lib/processor_test.go +++ b/pkg/lib/processor_test.go @@ -128,7 +128,7 @@ func (t *TelemetryProcessorTestSuite) TestCreateBundle() { } btags := types.Tags{types.Tag("key1=value1"), types.Tag("key2")} - bundleRow, berr := telemetryprocessor.GenerateBundle(1, "customer id", btags) + bundleRow, berr := telemetryprocessor.GenerateBundle("1", "customer id", btags) if berr != nil { t.Fail("Test failed to create the bundle") @@ -206,7 +206,7 @@ func (t *TelemetryProcessorTestSuite) TestReport() { // generate a bundle to hold unassigned items btags := types.Tags{types.Tag("key1=value1"), types.Tag("key2")} - bundleRow, berr := telemetryprocessor.GenerateBundle(1, "customer id", btags) + bundleRow, berr := telemetryprocessor.GenerateBundle("1", "customer id", btags) if berr != nil { t.Fail("Test failed to create the bundle") } @@ -239,7 +239,7 @@ func (t *TelemetryProcessorTestSuite) TestReport() { // generate a second bundle btags1 := types.Tags{types.Tag("key3=value3"), types.Tag("key4")} - bundleRow, berr = telemetryprocessor.GenerateBundle(1, "customer id", btags1) + bundleRow, berr = telemetryprocessor.GenerateBundle("1", "customer id", btags1) if berr != nil { t.Fail("Test failed to create the bundle") } @@ -274,7 +274,7 @@ func (t *TelemetryProcessorTestSuite) TestReport() { // generate a report consuming available bundles rtags := types.Tags{types.Tag("key5=value5"), types.Tag("key6")} - reportRow, err := telemetryprocessor.GenerateReport(123456, rtags) + reportRow, err := telemetryprocessor.GenerateReport("123456", rtags) assert.NoError(t.T(), err, "Report failed") // validate the total and unassigned bundle counts diff --git a/pkg/lib/reports.go b/pkg/lib/reports.go index c168a84..5993fef 100644 --- a/pkg/lib/reports.go +++ b/pkg/lib/reports.go @@ -15,7 +15,7 @@ type TelemetryReport struct { Footer TelemetryReportFooter `json:"footer" validate:"required"` } -func NewTelemetryReport(clientId int64, tags types.Tags) *TelemetryReport { +func NewTelemetryReport(clientId string, tags types.Tags) *TelemetryReport { tr := new(TelemetryReport) // fill in header fields @@ -35,7 +35,7 @@ func NewTelemetryReport(clientId int64, tags types.Tags) *TelemetryReport { type TelemetryReportHeader struct { ReportId string `json:"reportId" validate:"required"` ReportTimeStamp string `json:"reportTimeStamp" validate:"required"` - ReportClientId int64 `json:"reportClientId" validate:"required"` + ReportClientId string `json:"reportClientId" validate:"required"` ReportAnnotations []string `json:"reportAnnotations"` } @@ -48,7 +48,7 @@ const reportsColumns = `( id INTEGER NOT NULL PRIMARY KEY, reportId VARCHAR(64) NOT NULL, reportTimestamp VARCHAR(32) NOT NULL, - reportClientId INTEGER NOT NULL, + reportClientId VARCHAR NOT NULL, reportAnnotations TEXT, reportChecksum VARCHAR(256) )` @@ -57,12 +57,12 @@ type TelemetryReportRow struct { Id int64 ReportId string ReportTimestamp string - ReportClientId int64 + ReportClientId string ReportAnnotations string ReportChecksum string } -func NewTelemetryReportRow(clientId int64, tags types.Tags) (*TelemetryReportRow, error) { +func NewTelemetryReportRow(clientId string, tags types.Tags) (*TelemetryReportRow, error) { report := NewTelemetryReport(clientId, tags) reportRow := TelemetryReportRow{} diff --git a/pkg/restapi/restapi.go b/pkg/restapi/restapi.go index d6eb01f..6c9c7b6 100644 --- a/pkg/restapi/restapi.go +++ b/pkg/restapi/restapi.go @@ -14,7 +14,7 @@ import ( // ClientRegistrationRequest is the request payload body POST'd to the server type ClientRegistrationRequest struct { - ClientInstanceId types.ClientInstanceId `json:"clientInstanceId"` + ClientRegistration types.ClientRegistration `json:"clientRegistration"` } func (c *ClientRegistrationRequest) String() string { @@ -25,7 +25,7 @@ func (c *ClientRegistrationRequest) String() string { // ClientRegistrationResponse is the response payload body from the server type ClientRegistrationResponse struct { - ClientId int64 `json:"clientId"` + RegistrationId int64 `json:"registrationId"` AuthToken string `json:"authToken"` RegistrationDate string `json:"registrationDate"` } @@ -38,8 +38,8 @@ func (c *ClientRegistrationResponse) String() string { // Client Authenticate handling via /temelemtry/authenticate type ClientAuthenticationRequest struct { - ClientId int64 `json:"clientId"` - InstIdHash types.ClientInstanceIdHash `json:"instIdHash"` + RegistrationId int64 `json:"registrationId"` + RegHash types.ClientRegistrationHash `json:"regHash"` } func (c *ClientAuthenticationRequest) String() string { diff --git a/pkg/types/types.go b/pkg/types/types.go index ec48992..80d9b0e 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -129,31 +129,37 @@ func (tc *TelemetryClass) String() string { return "UNKNOWN_TELEMETRY_CLASS" } -type ClientInstanceIdHash struct { +type ClientRegistrationHash struct { Method string `json:"method"` Value string `json:"value"` } -func (c *ClientInstanceIdHash) String() string { +func (c *ClientRegistrationHash) String() string { bytes, _ := json.Marshal(c) return string(bytes) } -func (c *ClientInstanceIdHash) Match(m *ClientInstanceIdHash) bool { +func (c *ClientRegistrationHash) Match(m *ClientRegistrationHash) bool { return (c.Method == m.Method) && (c.Value == m.Value) } // ClientInstanceId -type ClientInstanceId string +// type ClientRegistrationId string +type ClientRegistration struct { + ClientId string `json:"clientId"` + SystemUUID string `json:"systemUUID"` + Timestamp string `json:"timestamp"` +} -func (c *ClientInstanceId) String() string { - return string(*c) +func (c *ClientRegistration) String() string { + bytes, _ := json.Marshal(c) + return string(bytes) } const DEF_INSTID_HASH_METHOD = "sha256" -func (c *ClientInstanceId) Hash(inputMethod string) *ClientInstanceIdHash { +func (c *ClientRegistration) Hash(inputMethod string) *ClientRegistrationHash { var methodHash hash.Hash // this routine is expected to succeed so ensure a valid method is used @@ -179,10 +185,10 @@ func (c *ClientInstanceId) Hash(inputMethod string) *ClientInstanceIdHash { case "sha512": methodHash = sha512.New() } - methodHash.Write([]byte(*c)) + methodHash.Write([]byte(c.String())) // construct the return value - return &ClientInstanceIdHash{ + return &ClientRegistrationHash{ Method: method, Value: hex.EncodeToString(methodHash.Sum(nil)), } diff --git a/telemetry.go b/telemetry.go index 8fd262a..be424d3 100644 --- a/telemetry.go +++ b/telemetry.go @@ -142,7 +142,7 @@ const ( CLIENT_DISABLED CLIENT_MISCONFIGURED CLIENT_DATASTORE_ACCESSIBLE - CLIENT_INSTANCE_ID_ACCESSIBLE + CLIENT_REGISTRATION_ACCESSIBLE CLIENT_REGISTERED ) @@ -158,8 +158,8 @@ func (cs *ClientStatus) String() string { return "MISCONFIGURED" case CLIENT_DATASTORE_ACCESSIBLE: return "DATASTORE_ACCESSIBLE" - case CLIENT_INSTANCE_ID_ACCESSIBLE: - return "INSTANCE_ID_ACCESSIBLE" + case CLIENT_REGISTRATION_ACCESSIBLE: + return "REGISTRATION_ACCESSIBLE" case CLIENT_REGISTERED: return "REGISTERED" } @@ -205,13 +205,13 @@ func Status() (status ClientStatus) { status = CLIENT_DATASTORE_ACCESSIBLE // check that an instance id is available - if !tc.InstanceIdAccessible() { - slog.Warn("Telemetry client instance id has not been setup", slog.String("path", tc.InstIdPath())) + if !tc.RegistrationAccessible() { + slog.Warn("Telemetry client registration has not been setup", slog.String("path", tc.RegistrationPath())) return } // update status to indicate client has instance id - status = CLIENT_INSTANCE_ID_ACCESSIBLE + status = CLIENT_REGISTRATION_ACCESSIBLE // check that we have obtained a telemetry auth token if !tc.AuthAccessible() { From 463e8d3ae1d4f35de11c5dfd252b4c8d4f309e3f Mon Sep 17 00:00:00 2001 From: Fergal Mc Carthy Date: Thu, 6 Feb 2025 10:57:49 -0500 Subject: [PATCH 2/5] Correct logic flaw in client registration handling Need to ensure that the registration is loaded before checking for the authentication creds. Also correct some lingering references to client instance id. --- pkg/client/authenticate.go | 12 ++++++------ pkg/client/client.go | 5 ----- pkg/client/register.go | 12 ++++++------ pkg/types/types.go | 3 +-- telemetry.go | 4 ++-- 5 files changed, 15 insertions(+), 21 deletions(-) diff --git a/pkg/client/authenticate.go b/pkg/client/authenticate.go index e10265a..3b73565 100644 --- a/pkg/client/authenticate.go +++ b/pkg/client/authenticate.go @@ -15,6 +15,12 @@ import ( // Authenticate is responsible for (re)authenticating an already registered // client with the server to ensure that it's auth token is up to date. func (tc *TelemetryClient) Authenticate() (err error) { + // get the registration, failing if it can't be retrieved + regId, err := tc.getRegistration() + if err != nil { + return + } + if err = tc.loadTelemetryAuth(); err != nil { return fmt.Errorf( "telemetry client (re-)authentication requires an existing "+ @@ -23,12 +29,6 @@ func (tc *TelemetryClient) Authenticate() (err error) { ) } - // get the instanceId, failing if it can't be retrieved - regId, err := tc.getRegistration() - if err != nil { - return - } - // assemble the authentication request caReq := restapi.ClientAuthenticationRequest{ RegistrationId: tc.auth.RegistrationId, diff --git a/pkg/client/client.go b/pkg/client/client.go index 72fdb41..d3ed93a 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -22,7 +22,6 @@ const ( CONFIG_DIR = "/tmp/susetelemetry" CONFIG_PATH = CONFIG_DIR + "/config.yaml" AUTH_PATH = CONFIG_DIR + "/auth.json" - //INSTANCEID_PATH = CONFIG_DIR + "/instanceid" ) // TODO: unify with restapi.ClientRegistrationResponse @@ -187,10 +186,6 @@ func (tc *TelemetryClient) HasAuth() bool { return checkFileExists(tc.AuthPath()) } -func (tc *TelemetryClient) HasInstanceId() bool { - return checkFileExists(tc.RegistrationPath()) -} - func (tc *TelemetryClient) Processor() telemetrylib.TelemetryProcessor { // may want to just make the processor a public field return tc.processor diff --git a/pkg/client/register.go b/pkg/client/register.go index fa5c45c..ea3cd69 100644 --- a/pkg/client/register.go +++ b/pkg/client/register.go @@ -13,6 +13,12 @@ import ( ) func (tc *TelemetryClient) Register() (err error) { + // get the registration, failing if it can't be retrieved + reg, err := tc.getRegistration() + if err != nil { + return + } + // get the saved TelemetryAuth, returning success if found err = tc.loadTelemetryAuth() if err == nil { @@ -20,12 +26,6 @@ func (tc *TelemetryClient) Register() (err error) { return } - // get the registration, failing if it can't be retrieved - reg, err := tc.getRegistration() - if err != nil { - return - } - // register the system as a client var crReq restapi.ClientRegistrationRequest crReq.ClientRegistration = reg diff --git a/pkg/types/types.go b/pkg/types/types.go index 80d9b0e..1b27c63 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -144,8 +144,7 @@ func (c *ClientRegistrationHash) Match(m *ClientRegistrationHash) bool { return (c.Method == m.Method) && (c.Value == m.Value) } -// ClientInstanceId -// type ClientRegistrationId string +// ClientRegistration type ClientRegistration struct { ClientId string `json:"clientId"` SystemUUID string `json:"systemUUID"` diff --git a/telemetry.go b/telemetry.go index be424d3..9b6dd80 100644 --- a/telemetry.go +++ b/telemetry.go @@ -204,13 +204,13 @@ func Status() (status ClientStatus) { // update status to indicate that telemetry client datastore is accessible status = CLIENT_DATASTORE_ACCESSIBLE - // check that an instance id is available + // check that an registration is available if !tc.RegistrationAccessible() { slog.Warn("Telemetry client registration has not been setup", slog.String("path", tc.RegistrationPath())) return } - // update status to indicate client has instance id + // update status to indicate client has registration status = CLIENT_REGISTRATION_ACCESSIBLE // check that we have obtained a telemetry auth token From 2755e66fe8f99d3aa4de9c692b0f55930704050c Mon Sep 17 00:00:00 2001 From: Fergal Mc Carthy Date: Thu, 6 Feb 2025 15:43:10 -0500 Subject: [PATCH 3/5] Further registration management enhancements Primarily ensuring that any existing registrations are loaded before attempting to generate a new one. Also add support for handling an HTTP StatusConflict (409) in the case of a duplicate registration attempt by retrying registering the client once after triggering a regeneration of the client registration. --- pkg/client/client.go | 86 +++++++++++++++------------ pkg/client/register.go | 28 ++++++++- pkg/client/registration_management.go | 65 ++++++++++++++++---- 3 files changed, 129 insertions(+), 50 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index d3ed93a..7112135 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -71,12 +71,44 @@ func checkFileReadAccessible(filePath string) bool { return true } -func (tc *TelemetryClient) ensureRegistrationExists() (err error) { +func (tc *TelemetryClient) getRegistration() (reg types.ClientRegistration, err error) { + // ensure that a registration exists, creating one if needed + err = tc.ensureRegistrationExists() + if err != nil { + return + } + + // if a registration is already loaded then nothing more to do + if tc.reg.Valid() { + reg = tc.reg.Registration() + return + } + + err = tc.reg.Load() + if err != nil { + slog.Debug( + "failed to load client registration", + slog.String("reg", tc.reg.Path()), + slog.String("err", err.Error()), + ) + return + } + + slog.Debug( + "successfully loaded client registration", + slog.String("reg", tc.reg.String()), + ) - // if the client registration is valid and accessible then nothing to do - if tc.reg.Valid() && tc.reg.Accessible() { + reg = tc.reg.ClientRegistration + + return +} + +func (tc *TelemetryClient) ensureRegistrationExists() (err error) { + // if the existing client registration is valid then nothing to do + if tc.reg.Valid() { slog.Debug( - "client registration exists", + "client registration already loaded", slog.String("reg", tc.reg.String()), ) return nil @@ -87,19 +119,26 @@ func (tc *TelemetryClient) ensureRegistrationExists() (err error) { slog.String("regPath", tc.reg.path), ) - // generate a new client registration if needed - if !tc.reg.Valid() { - tc.reg.Generate() + if tc.reg.Accessible() { slog.Debug( - "client registration generated", - slog.String("reg", tc.reg.String()), + "client registration exists, needs to be loaded", + slog.String("regPath", tc.reg.Path()), ) + return nil } + // need to generate a new client registration if needed + tc.reg.Generate() + slog.Debug( + "client registration generated", + slog.String("reg", tc.reg.String()), + ) + + // save the generated client registration err = tc.reg.Save() if err != nil { slog.Debug( - "failed to save client registration generated", + "failed to save generated client registration", slog.String("reg", tc.reg.String()), ) return err @@ -208,33 +247,6 @@ func (tc *TelemetryClient) RegistrationAccessible() bool { return tc.reg.Accessible() } -func (tc *TelemetryClient) getRegistration() (reg types.ClientRegistration, err error) { - - err = tc.ensureRegistrationExists() - if err != nil { - return - } - - err = tc.reg.Load() - if err != nil { - slog.Debug( - "failed to load client registration", - slog.String("reg", tc.reg.Path()), - slog.String("err", err.Error()), - ) - return - } - - slog.Debug( - "successfully loaded client registration", - slog.String("reg", tc.reg.String()), - ) - - reg = tc.reg.ClientRegistration - - return -} - func (tc *TelemetryClient) deleteTelemetryAuth() (err error) { if err = os.Remove(tc.AuthPath()); err != nil { slog.Error( diff --git a/pkg/client/register.go b/pkg/client/register.go index ea3cd69..a1f4fe3 100644 --- a/pkg/client/register.go +++ b/pkg/client/register.go @@ -71,8 +71,32 @@ func (tc *TelemetryClient) Register() (err error) { return } - // TODO: Handle http.StatusConflict (409) as needing to regenerate registration - if resp.StatusCode != http.StatusOK { + // check the response status code, and handle appropriately + switch resp.StatusCode { + case http.StatusOK: + // all good, nothing to do + + case http.StatusConflict: + // retry if a duplicate client registration attempt is detected + if tc.reg.RetriesEnabled() { + slog.Warn( + "Duplicate client registration detected, forcing re-registration", + ) + + // delete the existing registration, forcing it to be regenerated as + // part of the next client registration attempt + tc.reg.Remove() + + // disable further retries + tc.reg.DisableRetries() + + // retry client registration + return tc.Register() + } + fallthrough + + default: + // unhandled error so fail appropriately err = fmt.Errorf("client registration failed: %s", string(respBody)) return } diff --git a/pkg/client/registration_management.go b/pkg/client/registration_management.go index 16f9a4e..9b49afa 100644 --- a/pkg/client/registration_management.go +++ b/pkg/client/registration_management.go @@ -18,17 +18,27 @@ const ( type TelemetryClientRegistration struct { types.ClientRegistration - path string - valid bool + path string + valid bool + no_retry bool } func NewTelemetryClientRegistration() *TelemetryClientRegistration { return &TelemetryClientRegistration{ - path: REGISTRATION_PATH, - valid: false, + path: REGISTRATION_PATH, + valid: false, + no_retry: false, } } +func (r *TelemetryClientRegistration) RetriesEnabled() bool { + return !r.no_retry +} + +func (r *TelemetryClientRegistration) DisableRetries() { + r.no_retry = true +} + func (r *TelemetryClientRegistration) Valid() bool { return r.valid } @@ -80,7 +90,7 @@ func (r *TelemetryClientRegistration) Save() (err error) { if err != nil { slog.Error( "failed to json.Marshal() client registration", - slog.String("regId", r.String()), + slog.String("reg", r.String()), slog.String("err", err.Error()), ) return @@ -91,7 +101,7 @@ func (r *TelemetryClientRegistration) Save() (err error) { if err != nil { slog.Error( "failed to write client registration file", - slog.String("regId", r.String()), + slog.String("reg", r.String()), slog.String("err", err.Error()), ) return @@ -99,7 +109,7 @@ func (r *TelemetryClientRegistration) Save() (err error) { slog.Debug( "client registration saved", - slog.String("regId", r.String()), + slog.String("reg", r.String()), ) return } @@ -117,7 +127,7 @@ func (r *TelemetryClientRegistration) Load() (err error) { } slog.Error( msg, - slog.String("regIdPath", r.path), + slog.String("regPath", r.path), slog.String("err", err.Error()), ) return @@ -129,7 +139,7 @@ func (r *TelemetryClientRegistration) Load() (err error) { if err != nil { slog.Error( "failed to read client registration file", - slog.String("regIdPath", r.path), + slog.String("regPath", r.path), slog.String("err", err.Error()), ) return @@ -141,7 +151,7 @@ func (r *TelemetryClientRegistration) Load() (err error) { if err != nil { slog.Error( "failed to json.Unmarshal() client registration file contents", - slog.String("regIdPath", r.path), + slog.String("regPath", r.path), slog.String("contents", string(bytes)), slog.String("err", err.Error()), ) @@ -150,7 +160,40 @@ func (r *TelemetryClientRegistration) Load() (err error) { slog.Debug( "client registration loaded", - slog.String("regId", r.String()), + slog.String("reg", r.String()), ) return } + +func (r *TelemetryClientRegistration) Remove() (err error) { + // mark in-memory version as invalid + r.valid = false + + // check if registration file exists + _, err = os.Stat(r.path) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + // nothing to do, and not a failure, if registration file doesn't exist + err = nil + } else { + slog.Error( + "unable to os.Stat() client registration file", + slog.String("regPath", r.path), + slog.String("err", err.Error()), + ) + } + return + } + + // remove the registration client, reporting any errors that occurr + err = os.Remove(r.path) + if err != nil { + slog.Error( + "failed to os.Remove() client registration file", + slog.String("regPath", r.path), + slog.String("err", err.Error()), + ) + } + + return +} From d9f45d249fda21513357b18d0183850afb161851 Mon Sep 17 00:00:00 2001 From: Fergal Mc Carthy Date: Fri, 7 Feb 2025 15:03:36 -0500 Subject: [PATCH 4/5] Updated docs to further reflect client registration changes --- doc/README.md | 3 +- doc/api/headers/telemetry-registration-id.md | 13 +++--- doc/api/requests/register.md | 4 +- doc/telemetryclientid.md | 12 ------ doc/telemetryclientregistration.md | 45 ++++++++++++++++++++ doc/telemetryregistrationid.md | 17 ++++++++ doc/telemetryrelay.md | 5 ++- doc/telemetrytag.md | 14 +++--- pkg/client/register.go | 5 +++ 9 files changed, 88 insertions(+), 30 deletions(-) delete mode 100644 doc/telemetryclientid.md create mode 100644 doc/telemetryclientregistration.md create mode 100644 doc/telemetryregistrationid.md diff --git a/doc/README.md b/doc/README.md index 4da7773..f0a109f 100644 --- a/doc/README.md +++ b/doc/README.md @@ -5,6 +5,7 @@ The following topics are documented: * [Telemetry Types](telemetrytype.md) * [Telemetry Tags](telemetrytag.md) * [Telemetry Timestamps](telemetrytimestamp.md) -* [Telemetry Client Ids](telemetryclientid.md) +* [Telemetry Client Registration](telemetryclientregistration.md) +* [Telemetry Registration Ids](telemetryregistrationid.md) * [Telemetry Relay](telemetryrelay.md) * [Telemetry Client REST API](api/) \ No newline at end of file diff --git a/doc/api/headers/telemetry-registration-id.md b/doc/api/headers/telemetry-registration-id.md index 1874e3d..c5fb598 100644 --- a/doc/api/headers/telemetry-registration-id.md +++ b/doc/api/headers/telemetry-registration-id.md @@ -1,9 +1,10 @@ -# Telemetry Client Id Header -When a telemetry client is submitting a telemetry report using -the [/report](../requests/report.md) request it will need to provide a -`X-Telemetry-Registration-Id` header specifying the registrationId from the client -credentials obtained using the [/register](../requests/register.md) request. +# Telemetry Registration Id Header +When a telemetry client is submitting a telemetry report using the +[/report](../requests/report.md) request it will need to provide a +`X-Telemetry-Registration-Id` header specifying the registrationId +from the client credentials obtained using the +[/register](../requests/register.md) request. -## Format of the Telemetry Client Id Header +## Format of the Telemetry Registration Id Header The `X-Telemetry-Registration-Id` header value should be the string representation of the int64 registrationId value. \ No newline at end of file diff --git a/doc/api/requests/register.md b/doc/api/requests/register.md index 87a0934..23c6eb0 100644 --- a/doc/api/requests/register.md +++ b/doc/api/requests/register.md @@ -15,7 +15,7 @@ Request body type `ClientRegistrationRequest` defined in [restapi module](../../ | Code | Description | Example | | ---- | ----------- | ------- | | 200 | Success
`Content-Type: application/json`
{
  registrationId integer($int64)
  authToken string
  registrationDate string
} | {
  "registrationId": 1234567890
  "authToken": "encoded.JWT.token"
  "registrationDate": "2024-08-01T01:02:03.000000Z"
} | -| 400 | Bad Request
Missing or incompatible body
`Content-Type: application/json`
{
  error string
} | {
  "error": "missing registration clientId"
}
or
{
  "error": "missing registration timespace"
} | -| 409 | Conflict
Client Registration already registered
`Content-Type: application/json`
{
  error string
} | {
  "error": "specified registration already exists"
} | +| 400 | Bad Request
Missing or incompatible body
`Content-Type: application/json`
{
  error string
} | {
  "error": "missing registration clientId"
}
or
{
  "error": "missing registration timestamp"
} | +| 409 | Conflict
Client Registration or Registration's Client Id already registered
`Content-Type: application/json`
{
  error string
} | {
  "error": "specified registration already exists"
}
or
{
  "error": "specified registration clientId already exists"
} | Response success body type `ClientRegistrationResponse` defined in [restapi module](../../../pkg/restapi/) \ No newline at end of file diff --git a/doc/telemetryclientid.md b/doc/telemetryclientid.md deleted file mode 100644 index d274046..0000000 --- a/doc/telemetryclientid.md +++ /dev/null @@ -1,12 +0,0 @@ -# Telemetry Client Ids - -A telemetry server (or relay) manages a pool of telemetry client ids, -current ranging from 1 to MAX_INT64. - -When a client [registers](api/requests/register.md) with an upstream -telemetry server (or relay) it is assigned a new client id. - -This telemetry client id only has meaning in the context of a specific -combination of telemetry client and telemetry server, and the same -client id can be assigned to multiple telemetry clients so long as they -are talking to different upstream telemery servers (or relays). \ No newline at end of file diff --git a/doc/telemetryclientregistration.md b/doc/telemetryclientregistration.md new file mode 100644 index 0000000..4fad4f5 --- /dev/null +++ b/doc/telemetryclientregistration.md @@ -0,0 +1,45 @@ +# Telemetry Client Registration + +To submit telemetry reports to an upstream telemetry service (gateway +or relay) a client must [register](api/requests/register.md) with that +service. As part of the registration process a client will generate a +client registration structure which is used to uniquely identify it as +a telemetry service client. + +## Client Registration Structure +This client registration structure has the following format: +``` +{ + "clientId": "", + "systemUUID": "", + "timestamp": "" +} +``` + +## Client Registration Collisions +A client system may be required to generate a new client registration +if the upstream server detects that the registration is a duplicate +of an existing registration, or that the registration's clientId is +a duplicate of existing client's clientId. + +The client registration's clientId value is used to identify the +client generating a [telemetry bundle](api/structs/telemetrybundle.md) +and submitting a [telemetry report](api/structs/telemetryreport.md). + +## Uniquely Identifying a client +On its own the clientId may not always uniquely identify a client +within the overall pool of telemetry service submissions, because two +client systems could independently generate the same UUID value to use +as a clientId, but a telemetry client's clientId will always be unique +with respect to other clients of the same upstream telemetry service. + +This property, that clientIds will always be unique with respect to +other clients of the same upstream telemetry service, can be leveraged +by [telemetry relays](telemetryrelay.md) to assist in uniquely +identifying telemetry clients. When telemetry is relayed, the relay +will add a RELAYED_VIA tag to the telemetry submission which identifies +both the relay and the client that submitted the telemetry to the +relay. The aggregate of the RELATED_VIA tag values associated with a +telemetry data item can thus be used to uniquely identify the path +from the originating client to the main telemetry service gateway, +and thus uniquely identify a specific client's telemetry submissions. diff --git a/doc/telemetryregistrationid.md b/doc/telemetryregistrationid.md new file mode 100644 index 0000000..36e1317 --- /dev/null +++ b/doc/telemetryregistrationid.md @@ -0,0 +1,17 @@ +# Telemetry Registration Ids + +A telemetry server (or relay) manages a pool of telemetry client +registration ids, currently ranging from 1 to MAX_INT64. + +Once a client has successfully [registered](api/requests/register.md) +it's [client registration](telemetryclientregistration.md) with an +upstream telemetry service the response will contain the client's +credentials, including the registrationId which will be used to set +the [X-Telemetry-Registration-Id](api/headers/telemetry-registration-id.md) +when submitting telemetry requests to the upstream telemetry service. + +Note that this telemetry registration id only has meaning in the +context of a specific combination of telemetry client and telemetry +server, and the same registration id may be assigned to multiple +telemetry clients so long as they are talking to different upstream +telemery servers (or relays). \ No newline at end of file diff --git a/doc/telemetryrelay.md b/doc/telemetryrelay.md index 5c5e4ff..11c2d38 100644 --- a/doc/telemetryrelay.md +++ b/doc/telemetryrelay.md @@ -15,8 +15,9 @@ will be processed as follows: the [telemetry bundles](api/structs/telemetrybundle.md). 2. The telemetry relay will annotate each bundle with a new [RELAYED_VIA tag](telemetrytag.md) whose value consists of the - [client Id](telemetryclientid.md) of the client that submitted the report, and the telemetry - relay's own client Id, joined with a `:`. + [registration Id](telemetryregistrationid.md) of the client that + submitted the report, and the telemetry relay's own registration Id, + joined with a `:`. 3. The telemetry relay will stage the received telemetry bundles locally. 4. Once sufficient bundles are available, or enough time has passed, diff --git a/doc/telemetrytag.md b/doc/telemetrytag.md index 6a4e804..b6c09e8 100644 --- a/doc/telemetrytag.md +++ b/doc/telemetrytag.md @@ -29,12 +29,12 @@ inheritance rules: ## Telemetry Annotation Tag Examples Some examples: -* when bundles are relayed via a telemetry relay server a RELAYED_VIA - tag composed of the combination of the client id of the reporting - client and the relay server's client id will be added to the relayed - bundles. -* telemetry relayed via a proxy service may be annotated by the proxy - type, e.g +* when bundles are relayed via a [telemetry relay](telemetryrelay.md) + server a RELAYED_VIA tag composed of the combination of the registration + id of the reporting client and the relay server's registration id will + be added to the relayed bundles. +* telemetry relayed or synthesised via a proxy service could be annotated + by the proxy type, e.g * PROXY_TYPE=RMT for RMT - * PROXY_TYPE=SUMA for SUSE Manager + * PROXY_TYPE=MLM for Multi Linux Manager * PROXY_TYPE=SCC for SCC \ No newline at end of file diff --git a/pkg/client/register.go b/pkg/client/register.go index a1f4fe3..fe3c220 100644 --- a/pkg/client/register.go +++ b/pkg/client/register.go @@ -77,6 +77,11 @@ func (tc *TelemetryClient) Register() (err error) { // all good, nothing to do case http.StatusConflict: + slog.Debug( + "StatusConflict returned", + slog.Int("StatusCode", resp.StatusCode), + slog.String("error", string(respBody)), + ) // retry if a duplicate client registration attempt is detected if tc.reg.RetriesEnabled() { slog.Warn( From 658c822c5671785b633b306d2d0b6857d8dd68b1 Mon Sep 17 00:00:00 2001 From: Fergal Mc Carthy Date: Wed, 12 Feb 2025 08:57:21 -0500 Subject: [PATCH 5/5] Update doc/telemetryclientregistration.md Co-authored-by: Gabriel Silva Bueno --- doc/telemetryclientregistration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/telemetryclientregistration.md b/doc/telemetryclientregistration.md index 4fad4f5..2bc902d 100644 --- a/doc/telemetryclientregistration.md +++ b/doc/telemetryclientregistration.md @@ -39,7 +39,7 @@ by [telemetry relays](telemetryrelay.md) to assist in uniquely identifying telemetry clients. When telemetry is relayed, the relay will add a RELAYED_VIA tag to the telemetry submission which identifies both the relay and the client that submitted the telemetry to the -relay. The aggregate of the RELATED_VIA tag values associated with a +relay. The aggregate of the RELAYED_VIA tag values associated with a telemetry data item can thus be used to uniquely identify the path from the originating client to the main telemetry service gateway, and thus uniquely identify a specific client's telemetry submissions.