[HPR-1260] Support Project Level Service Principals Auth with HCP Packer #12520
Conversation
nywilken
changed the title
nywilken
force-pushed
the
nywilken/add-hcp-plsp-support
branch
from
0728e49
to
15aef8d
Compare
nywilken
changed the title
| } | ||
| // ValidateRegistryForProject validates that there is an active registry associated to the configured organization and project ids. | ||
| // A successful validation will result in a nil response. All other response represent an invalid registry error request or a registry not found error. | ||
| func (client *Client) ValidateRegistryForProject() error { |
There was a problem hiding this comment.
Interesting and good idea!
| } | ||
| } | ||
|
|
||
| return client, nil | ||
| } | ||
|
|
||
| func (c *Client) loadOrganizationID() error { | ||
| if env.HasOrganizationID() { |
There was a problem hiding this comment.
Regarding this check, doesn't that conflict with the comment left on line 71?
if both HCP_ORGANIZATION_ID and HCP_PROJECT_ID are set via env variables the hcpConfig may have all we need already.
If the client's Profile has them set already via the environment variable, is there a reason why we check for this here?
There was a problem hiding this comment.
Looking at the initialization code done in Canonicalize, I don't see where the org/project ID are loaded from the environment, is it possible that the Profile.OrganizationID or Profile.ProjectID are never set beforehand? If this is so, the check on line 72 seems to be always false, is my assumption correct?
There was a problem hiding this comment.
Hey great question! This took me for a little ride at first, as well, but if you take a deep dive into the Canonicalize call there is logic that builds the default UserProfile (Profile()) using the FromEnv configuration options. FromEnv will check the the local environment for a number of environment variables and set each one if the criteria for loading the env. variable is met. For some variables the rule is "If set use it" for others like HCP_ORGANIZATION_ID and HCP_PROJECT_ID they both must be set in order for the SDK to load them from the env. variable.
The SDK does not validate the values like we are doing for the Project ID but if the SDK logic changes in the future to do some validation and our Packer user sets both values we would benefit from the SDK doing the lifting. This is why I added the logic to check if set use. I'm checking the loading of the SDK because it may help us in the future when it comes to handling these vars consistently between products.
If the client's Profile has them set already via the environment variable, is there a reason why we check for this here?
As for this question the check is a guard clause if they are both set then we return and we don't check if they are set again. If only one is set then we have to check which is set and validate both accordingly.
Packer does not need the Organization ID set via a env. variable at this time. But I don't see any harm in adding it. In fact when I first wrote this code Packer loaded both but it was removed in favor of ListProjects and ListOrganization. Given the recent changes in service principles and how the platform is evolving my thinking is we should support the four main env variables until we can fully rely on the SDK for them.
Please let me know if that clears things up for you.
I will update the comment in the code to call out that they are only set by the SDK if both Project ID and Organization ID are set.
HCP supports two types of service principals: Organization-level and project-level. When a user tries to publish to an active HCP Packer registry using a plsp the client fails when configuring the client due to a API permission error; namely plsp do not have the permissions to query an org for a list of projects. Setting the HCP_PROJECT_ID does not resolve the issue because the call to ListProjects is still executed. This changes updates the client configuration params to obtain both the HCP Organization and Project IDs that will be used for connecting to the HCP Packer registry. With this change if a user provides a project Id via the HCP_PROJECT_ID environment variable no call to ListProjects will be made. Instead the value will be take as is and used to create the connection. A user connecting with a project level service principals must provide a valid HCP_PROJECT_ID in order to connect.
When setting a project id via the HCP_PROJECT_ID env the client will try to validate the project by checking that it has an associated registry. If the project is invalid or not a valid UUID an error will be displayed to the user * Add comment to clarify usage of SDK loaded env. variables
nywilken
force-pushed
the
nywilken/add-hcp-plsp-support
branch
from
3d30330
to
777c816
Compare
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
HCP supports two types of service principles: Organization-level and project-level.
When a user tries to publish to an active HCP Packer registry using a plsp the client
fails when configuring the client due to a API permission error; namely plsp do not have
the permissions to query an org for a list of projects. Setting the HCP_PROJECT_ID does
not resolve the issue because the call to ListProjects is still executed.
This changes updates the client configuration params to obtain both the HCP Organization and
Project IDs that will be used for connecting to the HCP Packer registry. With this change
if a user provides a project Id via the HCP_PROJECT_ID environment variable no call to ListProjects will
be made. Instead the value will be take as is and used to create the connection. A user connecting with
a project level service principle must provide a valid HCP_PROJECT_ID in order to connect.
Example Outputs: