Skip to content

Commit

Permalink
Rework the client registration workflow and client id
Browse files Browse the repository at this point in the history
Previously the client id value in telemetry reports and bundles was
the unique id assigned to a telemery client when it registered with
and upstream telemetry service, whether gateway or relay. With these
changes the client id becomes an UUID string generated by telemetry
clients when they first register with an upstream telemetry server.

Previously when a telemetry client was registering with an upstream
telemetry service it will include a client instance id arbitrary
string value that was intended to uniquely identify the client. With
this update, telemetry clients will instead generated a UUID value,
their client id, to be included in their registration requests, along
with a timestamp and an option systemUUID value. The latter fields
can be used to differentiate between telemetry clients that have
generated the same client id value, in those rare cases where this
happens.

Then result of a regsiter request now includes a unique registration
id value, rather than a client id, and subsequent requests from the
client should include this value as an X-Telemetry-Registration-Id
header value rather than the X-Telemetry-Client-Id header.

The clients table schema has been updated to store the client id,
systemUUID and timestamp values supplied as part of a register
request.

The telemetry table schema has been updated to store the client id
as a string value, rather than an integer.

Updated the GitHub PR testing workflow to allow specifying the
branch of the SUSE/telemetry to use when running tests.

TelemetryRepoBranch: client_customer_id_refinements
  • Loading branch information
rtamalin committed Feb 6, 2025
1 parent 796a716 commit 7021f64
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 87 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,23 @@ jobs:
with:
path: telemetry-server

- name: Select Telemetry Repo Branch
env:
PR_BODY: "${{ github.event.pull_request.body }}"
run: |
branch="$(echo "${PR_BODY}" | \
grep -o "^[[:space:]]*TelemetryRepoBranch: .*$" | \
cut -d":" -f2 | tr -d '[[:space:]]' || true)"
[[ -n "${branch}" ]] || branch=main
echo "TelemetryRepoBranch=${branch}" >> "$GITHUB_OUTPUT"
- name: Checkout SUSE/telemetry companion sources
uses: actions/checkout@v4
with:
repository: SUSE/telemetry
path: telemetry
ref: ${{ steps.Select_Telemetry_Repo_Branch.outputs.TelemetryRepoBranch }}

- name: Run tests in verbose mode
run: cd telemetry-server && make test-verbose
4 changes: 2 additions & 2 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ func (ar *AppRequest) GetAuthToken() string {
return strings.TrimPrefix(ar.GetAuthorization(), "Bearer ")
}

func (ar *AppRequest) GetClientId() string {
return ar.GetHeader("X-Telemetry-Client-Id")
func (ar *AppRequest) GetRegistrationId() string {
return ar.GetHeader("X-Telemetry-Registration-Id")
}

func (ar *AppRequest) SetHeader(header, value string) {
Expand Down
87 changes: 63 additions & 24 deletions app/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ var clientsTableSpec = TableSpec{
Name: "clients",
Columns: []TableSpecColumn{
{Name: "id", Type: "INTEGER", PrimaryKey: true, Identity: true},
{Name: "clientInstanceId", Type: "VARCHAR"},
{Name: "clientId", Type: "VARCHAR"},
{Name: "systemUUID", Type: "VARCHAR"},
{Name: "clientTimestamp", Type: "VARCHAR"},
{Name: "registrationDate", Type: "VARCHAR"},
{Name: "authToken", Type: "VARCHAR"},
},
Expand All @@ -23,26 +25,38 @@ type ClientsRow struct {
// include common table row fields
TableRowCommon

Id int64 `json:"id"`
ClientInstanceId types.ClientInstanceId `json:"clientInstanceId"`
RegistrationDate string `json:"registrationDate"`
AuthToken string `json:"authToken"`
Id int64 `json:"id"`
ClientId string `json:"clientId"`
SystemUUID string `json:"systemUUID"`
ClientTimestamp string `json:"clientTimestamp"`
RegistrationDate string `json:"registrationDate"`
AuthToken string `json:"authToken"`
}

func (c *ClientsRow) InitAuthentication(caReq *restapi.ClientAuthenticationRequest) {
c.InitClientId(caReq.ClientId)
c.InitRegistrationId(caReq.RegistrationId)
}

func (c *ClientsRow) InitClientId(clientId int64) {
c.Id = clientId
func (c *ClientsRow) InitRegistrationId(registrationId int64) {
c.Id = registrationId
}

func (c *ClientsRow) InitRegistration(crReq *restapi.ClientRegistrationRequest) {
c.InitClientInstanceId(&crReq.ClientInstanceId)
c.InitClientRegistration(&crReq.ClientRegistration)
}

func (c *ClientsRow) InitClientInstanceId(instId *types.ClientInstanceId) {
c.ClientInstanceId = *instId
func (c *ClientsRow) InitClientRegistration(reg *types.ClientRegistration) {
c.ClientId = reg.ClientId
c.SystemUUID = reg.SystemUUID
c.ClientTimestamp = reg.Timestamp
}

func (c *ClientsRow) GetClientRegistration() *types.ClientRegistration {
return &types.ClientRegistration{
ClientId: c.ClientId,
SystemUUID: c.SystemUUID,
Timestamp: c.ClientTimestamp,
}
}

func (c *ClientsRow) TableName() string {
Expand All @@ -67,7 +81,9 @@ func (c *ClientsRow) Exists() bool {
stmt, err := c.SelectStmt(
// select columns
[]string{
"clientInstanceId",
"clientId",
"systemUUID",
"clientTimestamp",
"registrationDate",
"authToken",
},
Expand All @@ -90,15 +106,19 @@ func (c *ClientsRow) Exists() bool {
// if the entry was found, all fields not used to find the entry will have
// been updated to match what is in the DB
if err := row.Scan(
&c.ClientInstanceId,
&c.ClientId,
&c.SystemUUID,
&c.ClientTimestamp,
&c.RegistrationDate,
&c.AuthToken,
); err != nil {
if err != sql.ErrNoRows {
slog.Error(
"check for matching entry failed",
slog.String("table", c.TableName()),
slog.String("clientInstanceId", c.ClientInstanceId.String()),
slog.String("clientId", c.ClientId),
slog.String("systemUUID", c.SystemUUID),
slog.String("clientTimestamp", c.ClientTimestamp),
slog.String("error", err.Error()),
)
}
Expand All @@ -107,7 +127,7 @@ func (c *ClientsRow) Exists() bool {
return true
}

func (c *ClientsRow) InstIdExists() bool {
func (c *ClientsRow) RegistrationExists() bool {
stmt, err := c.SelectStmt(
// select columns
[]string{
Expand All @@ -117,20 +137,27 @@ func (c *ClientsRow) InstIdExists() bool {
},
// match columns
[]string{
"clientInstanceId",
"clientId",
"systemUUID",
"clientTimestamp",
},
SelectOpts{}, // no special options
)
if err != nil {
slog.Error(
"instIdExists statement generation failed",
"registrationExists statement generation failed",
slog.String("table", c.TableName()),
slog.String("error", err.Error()),
)
panic(err)
}

row := c.DB().QueryRow(stmt, c.ClientInstanceId)
row := c.DB().QueryRow(
stmt,
c.ClientId,
c.SystemUUID,
c.ClientTimestamp,
)
// if the entry was found, all fields not used to find the entry will have
// been updated to match what is in the DB
if err := row.Scan(
Expand All @@ -142,7 +169,9 @@ func (c *ClientsRow) InstIdExists() bool {
slog.Error(
"check for matching entry failed",
slog.String("table", c.TableName()),
slog.String("clientInstanceId", c.ClientInstanceId.String()),
slog.String("clientId", c.ClientId),
slog.String("systemUUID", c.SystemUUID),
slog.String("clientTimestamp", c.ClientTimestamp),
slog.String("error", err.Error()),
)
}
Expand All @@ -154,7 +183,9 @@ func (c *ClientsRow) InstIdExists() bool {
func (c *ClientsRow) Insert() (err error) {
stmt, err := c.InsertStmt(
[]string{
"clientInstanceId",
"clientId",
"systemUUID",
"clientTimestamp",
"registrationDate",
"authToken",
},
Expand All @@ -170,7 +201,9 @@ func (c *ClientsRow) Insert() (err error) {
}
row := c.DB().QueryRow(
stmt,
c.ClientInstanceId,
c.ClientId,
c.SystemUUID,
c.ClientTimestamp,
c.RegistrationDate,
c.AuthToken,
)
Expand All @@ -180,7 +213,9 @@ func (c *ClientsRow) Insert() (err error) {
slog.Error(
"insert failed",
slog.String("table", c.TableName()),
slog.Any("clientInstanceId", c.ClientInstanceId),
slog.String("clientId", c.ClientId),
slog.String("systemUUID", c.SystemUUID),
slog.String("clientTimestamp", c.ClientTimestamp),
slog.String("error", err.Error()),
)
}
Expand All @@ -191,7 +226,9 @@ func (c *ClientsRow) Insert() (err error) {
func (c *ClientsRow) Update() (err error) {
stmt, err := c.UpdateStmt(
[]string{
"clientInstanceId",
"clientId",
"systemUUID",
"clientTimestamp",
"registrationDate",
"authToken",
},
Expand All @@ -209,7 +246,9 @@ func (c *ClientsRow) Update() (err error) {
}
_, err = c.DB().Exec(
stmt,
c.ClientInstanceId,
c.ClientId,
c.SystemUUID,
c.ClientTimestamp,
c.RegistrationDate,
c.AuthToken,
c.Id,
Expand Down
16 changes: 8 additions & 8 deletions app/handler_authenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ func (a *App) AuthenticateClient(ar *AppRequest) {
ar.ErrorResponse(http.StatusBadRequest, err.Error())
return
}
if caReq.ClientId <= 0 {
if caReq.RegistrationId <= 0 {
ar.SetWwwAuthRegister()
ar.ErrorResponse(http.StatusUnauthorized, "Invalid ClientId value provided")
ar.ErrorResponse(http.StatusUnauthorized, "Invalid registrationId value provided")
return
}
ar.Log.Debug("Unmarshaled", slog.Any("caReq", &caReq))
Expand All @@ -52,13 +52,13 @@ func (a *App) AuthenticateClient(ar *AppRequest) {
return
}

// confirm that the provided clientInstanceId SHA matches the registered one
instIdHash := client.ClientInstanceId.Hash(caReq.InstIdHash.Method)
if !instIdHash.Match(&caReq.InstIdHash) {
// confirm that the provided registration hash matches the registered one
regHash := client.GetClientRegistration().Hash(caReq.RegHash.Method)
if !regHash.Match(&caReq.RegHash) {
ar.Log.Error(
"ClientInstanceId hash mismatch",
slog.String("Req Hash", caReq.InstIdHash.String()),
slog.String("DB Hash", instIdHash.String()),
slog.String("Req Hash", caReq.RegHash.String()),
slog.String("DB Hash", regHash.String()),
)
// client needs to re-register
ar.SetWwwAuthRegister()
Expand All @@ -84,7 +84,7 @@ func (a *App) AuthenticateClient(ar *AppRequest) {

// initialise a client registration response
caResp := restapi.ClientAuthenticationResponse{
ClientId: client.Id,
RegistrationId: client.Id,
AuthToken: client.AuthToken,
RegistrationDate: client.RegistrationDate,
}
Expand Down
15 changes: 10 additions & 5 deletions app/handler_register.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ func (a *App) RegisterClient(ar *AppRequest) {
ar.ErrorResponse(http.StatusBadRequest, err.Error())
return
}
if string(crReq.ClientInstanceId) == "" {
ar.ErrorResponse(http.StatusBadRequest, "no ClientInstanceId value provided")
// verify that clientId and timestamp are specified in registration
if string(crReq.ClientRegistration.ClientId) == "" {
ar.ErrorResponse(http.StatusBadRequest, "missing registration clientId")
return
}
if string(crReq.ClientRegistration.Timestamp) == "" {
ar.ErrorResponse(http.StatusBadRequest, "missing registration timestamp")
return
}
ar.Log.Debug("Unmarshaled", slog.Any("crReq", &crReq))
Expand All @@ -44,8 +49,8 @@ func (a *App) RegisterClient(ar *AppRequest) {
}

client.InitRegistration(&crReq)
if client.InstIdExists() {
ar.ErrorResponse(http.StatusConflict, "specified clientInstanceId already exists")
if client.RegistrationExists() {
ar.ErrorResponse(http.StatusConflict, "specified registration already exists")
return
}

Expand All @@ -63,7 +68,7 @@ func (a *App) RegisterClient(ar *AppRequest) {

// initialise a client registration response
crResp := restapi.ClientRegistrationResponse{
ClientId: client.Id,
RegistrationId: client.Id,
AuthToken: client.AuthToken,
RegistrationDate: client.RegistrationDate,
}
Expand Down
20 changes: 10 additions & 10 deletions app/handler_report.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ func (a *App) ReportTelemetry(ar *AppRequest) {
ar.Log.Info("Processing")

// retrieve required headers
hdrClientId := ar.GetClientId()
hdrRegistrationId := ar.GetRegistrationId()
token := ar.GetAuthToken()

// missing clientId or token suggests client needs to register
if (hdrClientId == "") || (token == "") {
// missing registrationId or token suggests client needs to register
if (hdrRegistrationId == "") || (token == "") {
// client needs to register
ar.SetWwwAuthRegister()
ar.ErrorResponse(http.StatusUnauthorized, "Client registration required")
Expand All @@ -40,12 +40,12 @@ func (a *App) ReportTelemetry(ar *AppRequest) {
slog.String("token", token),
)

// verify that the provided client id is a valid number
clientId, err := strconv.ParseInt(hdrClientId, 0, 64)
// verify that the provided registration id is a valid number
registrationId, err := strconv.ParseInt(hdrRegistrationId, 0, 64)
if err != nil {
// client needs to register
ar.SetWwwAuthRegister()
ar.ErrorResponse(http.StatusUnauthorized, "Invalid Client Id")
ar.ErrorResponse(http.StatusUnauthorized, "Invalid Registration Id")
return
}

Expand All @@ -57,11 +57,11 @@ func (a *App) ReportTelemetry(ar *AppRequest) {
return
}

client.InitClientId(clientId)
client.InitRegistrationId(registrationId)
if !client.Exists() {
// client needs to register
ar.SetWwwAuthRegister()
ar.ErrorResponse(http.StatusUnauthorized, "Invalid Client Id")
ar.ErrorResponse(http.StatusUnauthorized, "Invalid Registration Id")
return
}

Expand All @@ -76,8 +76,8 @@ func (a *App) ReportTelemetry(ar *AppRequest) {
}

ar.Log.Debug(
"Client Authorizated",
slog.Int64("clientId", clientId),
"Client Authorized",
slog.Int64("registrationId", registrationId),
)

// handle payload compression
Expand Down
2 changes: 1 addition & 1 deletion app/staging.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (a *App) StageTelemetryReport(reqBody []byte, rHeader *telemetrylib.Telemet
}

reportStagingRow.Init(
fmt.Sprintf("%d", rHeader.ReportClientId),
fmt.Sprintf("%q", rHeader.ReportClientId),
rHeader.ReportId,
reqBody,
)
Expand Down
Loading

0 comments on commit 7021f64

Please sign in to comment.