-
Notifications
You must be signed in to change notification settings - Fork 59
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.
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
feat: azapi_client_config - add object id #657
feat: azapi_client_config - add object id #657
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @matt-FFFFFF for this PR. It mostly LGTM, just some minor comments, please check.
Hi @ms-henglu I have updated this and responded to your comments. Please can you review again? Thanks! |
internal/clients/account.go
Outdated
log.Printf("[DEBUG] Error getting requesting token from credentials: %s", err) | ||
} | ||
|
||
cl, err := parseTokenClaims(tok.Token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a nil-check on the tok
? If above GetToken returns error, the tok
could be nil here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tok
is of type AccessToken. This is a value type not a pointer.
The type declaration is:
type AccessToken struct {
Token string
ExpiresOn time
}
I refactored using a comparison of tok.Token == ""
as this will be the default value if the token request fails.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! LGTM
This PR closes the gap to the azurerm equivalent by adding the
object_id
attribute to theazapi_client_config
resource.Also adds a convenience attribute to azapi_client_config to allow for adding the subscription resource id easily into
parent_id
.@ms-henglu