Skip to content

feat: Fetch service account OIDC data #402

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
bec-callow-oct merged 1 commit into from
Feb 26, 2026
Merged

feat: Fetch service account OIDC data #402

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions pkg/serviceaccounts/oidc_identities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import (
)

type OIDCIdentityQuery struct {
Skip int `uri:"skip,omitempty" url:"skip,omitempty"`
Take int `uri:"take,omitempty" url:"take,omitempty"`
ServiceAccountId string `uri:"serviceAccountId" url:"serviceAccountId"`
Copy link
Copy Markdown
Contributor

@Rose-Northey Rose-Northey Feb 25, 2026

Choose a reason for hiding this comment

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

Would adding this field break old usages of the query if they were wotking?

Copy link
Copy Markdown
Contributor Author

@bec-callow-oct bec-callow-oct Feb 25, 2026

Choose a reason for hiding this comment

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

If there were working usages of this query, adding ServiceAccountId would not break them since it would default to empty string, but this query was only used for the GetOIDCIdentities function which was calling api/serviceaccounts//oidcidentities/v1{?skip,take} when the service account ID was not provided (note the double forward slash) and that endpoint does not exist on Octopus server.

The correct endpoint is api/serviceaccounts/{serviceAccountId}/oidcidentities/v1{?skip,take} where service account ID, Skip and Take are all required

Copy link
Copy Markdown
Contributor Author

@bec-callow-oct bec-callow-oct Feb 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe if someone had used the query to build a custom request so that they could bypass the broken function, there could be a working use of this and I guess we can't exactly predict whether the changes could break custom code. I still think this is a worthwhile change, but could release it as a potentially breaking change

Copy link
Copy Markdown
Contributor

@Rose-Northey Rose-Northey Feb 26, 2026

Choose a reason for hiding this comment

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

Yeah makes sense. Seems like the custom code situation would be super edge case

Skip int `uri:"skip" url:"skip"`
Take int `uri:"take" url:"take"`
}

type OIDCIdentity struct {
Expand All @@ -22,6 +23,13 @@ type OIDCIdentity struct {
resources.Resource
}

type ServiceAccountOIDCIdentitiesResponse struct {
ServerUrl string `json:"ServerUrl"`
ExternalId string `json:"ExternalId"`
OidcIdentities []*OIDCIdentity `json:"OidcIdentities"`
Count int `json:"Count"`
}

// NewOIDCIdentity initializes a Service Account with required fields.
func NewOIDCIdentity(serviceAccountID string, name string, issuer string, subject string) *OIDCIdentity {
return &OIDCIdentity{
Expand Down Expand Up @@ -77,6 +85,30 @@ func GetOIDCIdentities(client newclient.Client, query OIDCIdentityQuery) (*resou
return res, nil
}

// GetServiceAccountOIDCData queries the service account and identities for the provided service account ID
func GetServiceAccountOIDCData(client newclient.Client, query OIDCIdentityQuery) (*ServiceAccountOIDCIdentitiesResponse, error) {
if internal.IsEmpty(query.ServiceAccountId) {
return nil, internal.CreateInvalidParameterError("GetServiceAccountOIDCData", "query.ServiceAccountId")
}

values, _ := uritemplates.Struct2map(query)
if values == nil {
values = map[string]any{}
}

path, err := client.URITemplateCache().Expand(serviceAccountOIDCIDQueryTemplate, values)
if err != nil {
return nil, err
}

res, err := newclient.Get[ServiceAccountOIDCIdentitiesResponse](client.HttpSession(), path)
if err != nil {
return nil, err
}

return res, nil
}

Copy link
Copy Markdown
Contributor

@Rose-Northey Rose-Northey Feb 25, 2026

Choose a reason for hiding this comment

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

I'm guessing you've tested these with terraform. But is there a reason its difficult to have tests in go as well? Is it because of the difficulty of creating a service account for it? Is a service account the account you have with an auth application? so you'd need to simmulate an account with an external application?

Copy link
Copy Markdown
Contributor Author

@bec-callow-oct bec-callow-oct Feb 26, 2026

Choose a reason for hiding this comment

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

Yes, I've tested manually, both using the client directly and via terraform. There are no existing tests for OIDC identities to work from and the setup looks fairly complex

// GetOIDCIdentityByID queries OIDC identities by ID for the provided service account ID
func GetOIDCIdentityByID(client newclient.Client, serviceAccountID string, ID string) (*OIDCIdentity, error) {
path, err := client.URITemplateCache().Expand(serviceAccountOIDC, map[string]any{
Expand Down
Loading