Skip to content
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

Creating an optional interface to get secret id references for driver with secrets. #7306

5 changes: 5 additions & 0 deletions pkg/recipes/driver/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,11 @@ func (d *terraformDriver) GetRecipeMetadata(ctx context.Context, opts BaseOption
return recipeData, nil
}

func (d *terraformDriver) FindSecretIDs(ctx context.Context, envConfig recipes.Configuration, definition recipes.EnvironmentDefinition) (string, error) {

Choose a reason for hiding this comment

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

nit: Please add comment.


return recipes.GetSecretStoreID(envConfig, definition.TemplatePath)
}

// getDeployedOutputResources is used to the get the resource IDs by parsing the terraform state for resource information and using it to create UCP qualified IDs.
// Currently only Azure, AWS and Kubernetes providers are supported by output resources.
func (d *terraformDriver) getDeployedOutputResources(ctx context.Context, module *tfjson.StateModule) ([]string, error) {
Expand Down
7 changes: 7 additions & 0 deletions pkg/recipes/driver/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ type Driver interface {
GetRecipeMetadata(ctx context.Context, opts BaseOptions) (map[string]any, error)
}

type DriverWithSecrets interface {

Choose a reason for hiding this comment

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

nit: Please add comment

Driver

// FindSecretIDs gets the secret ID references from environment definition.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add more details in this comment? What is the secret ID for example? Where does it read it from in the environment definition - recipe config. Why is it needed - to access private modules (for now, with providers change it would extend to secrets passed into the providers config as well)

FindSecretIDs(ctx context.Context, config recipes.Configuration, definition recipes.EnvironmentDefinition) (string, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this return ([]string, error)?

What would happen in the future if a driver needed to read multiple secrets?

Copy link
Contributor Author

@vishwahiremat vishwahiremat Mar 13, 2024

Choose a reason for hiding this comment

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

currently we are keeping it to return module secret. But in the future when we have provider secrets etc, return type will be map because we may need to specify secret kind then ex: module secret or provider secret. It will be discussed in detail during the design for provider secrets.

}

// BaseOptions is the base options for the driver operations.
type BaseOptions struct {
// Configuration is the configuration for the recipe.
Expand Down
22 changes: 11 additions & 11 deletions pkg/recipes/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,19 @@ func (e *engine) executeCore(ctx context.Context, recipe recipes.ResourceMetadat
return nil, nil, err
}

// Retrieves the secret store id from the recipes configuration for the terraform module source of type git.
// secretStoreID returned will be an empty string for other types.
secretStore, err := recipes.GetSecretStoreID(*configuration, definition.TemplatePath)
if err != nil {
return nil, nil, err
}

// Retrieves the secret values from the secret store ID provided.
secrets := v20231001preview.SecretStoresClientListSecretsResponse{}
if secretStore != "" {
secrets, err = e.options.SecretsLoader.LoadSecrets(ctx, secretStore)
driverWithSecrets, ok := driver.(recipedriver.DriverWithSecrets)
if ok {
secretStore, err := driverWithSecrets.FindSecretIDs(ctx, *configuration, *definition)
if err != nil {
return nil, nil, fmt.Errorf("failed to fetch secrets from the secret store resource id %s for Terraform recipe %s deployment: %w", secretStore, definition.TemplatePath, err)
return nil, nil, err
}
// Retrieves the secret values from the secret store ID provided.
if secretStore != "" {
secrets, err = e.options.SecretsLoader.LoadSecrets(ctx, secretStore)
if err != nil {
return nil, nil, fmt.Errorf("failed to fetch secrets from the secret store resource id %s for Terraform recipe %s deployment: %w", secretStore, definition.TemplatePath, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need its own error code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, we could add a new error code for load secrets failures.
thoughts on naming it as LOAD_SECRETS_FAILED ?

Copy link
Contributor

Choose a reason for hiding this comment

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

ARM-like error codes PascalCase so it would be LoadSecretsFailed

}
}
}

Expand Down
Loading