feat(parametermanager): Added samples for get, list and render parameter & parameter version #5260
Conversation
…ter & parameter version
|
Here is the summary of changes. You are about to add 5 region tags.
This comment is generated by snippet-bot.
|
product-auto-label
bot
added
the
samples
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @durgesh-ninave-crest, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request introduces samples for interacting with Google Cloud Parameter Manager, specifically focusing on regional parameters and parameter versions. It includes code snippets for listing, viewing, and rendering parameters and their versions. The changes also include corresponding tests to ensure the functionality of the new samples. The go.mod and go.sum files are updated to reflect the new dependencies and versions required by the Parameter Manager SDK.
Highlights
- New Samples: Adds samples for listing all parameters, viewing parameter details, listing all parameter versions, viewing parameter version details, and rendering secret parameter versions.
- Regional Focus: The samples are designed to work with regional parameters, utilizing the
GOLANG_REGIONAL_SAMPLES_LOCATIONenvironment variable for configuration. - Dependency Updates: Updates
go.modandgo.sumto include the latest versions ofcloud.google.com/go/parametermanager,cloud.google.com/go/secretmanager, and other dependencies. - Testing: Includes new tests to validate the functionality of the added samples, ensuring they work as expected.
Changelog
Click here to see the changelog
- parametermanager/go.mod
- Updates the Go version to 1.23.0.
- Adds
cloud.google.com/go/secretmanagerandgithub.com/gofrs/uuidas dependencies. - Updates
cloud.google.com/go/parametermanagerto v0.2.0 andgoogle.golang.org/apito v0.228.0. - Updates indirect dependencies to their latest versions.
- parametermanager/go.sum
- Updates checksums for all dependencies to match the new versions in
go.mod.
- Updates checksums for all dependencies to match the new versions in
- parametermanager/regional_samples/get_regional_param.go
- Adds a new function
getRegionalParamto retrieve a regional parameter using the Parameter Manager SDK. - Includes code to create a Parameter Manager client with a regional endpoint.
- Formats the output to display the parameter name and format.
- Adds a new function
- parametermanager/regional_samples/get_regional_param_version.go
- Adds a new function
getRegionalParamVersionto retrieve a regional parameter version. - Includes code to create a Parameter Manager client with a regional endpoint.
- Formats the output to display the parameter version name, state (enabled/disabled), and payload (if enabled).
- Adds a new function
- parametermanager/regional_samples/list_regional_param.go
- Adds a new function
listRegionalParamto list all regional parameters. - Includes code to create a Parameter Manager client with a regional endpoint.
- Iterates through the parameters and formats the output to display the parameter name and format.
- Adds a new function
- parametermanager/regional_samples/list_regional_param_version.go
- Adds a new function
listRegionalParamVersionto list all regional parameter versions. - Includes code to create a Parameter Manager client with a regional endpoint.
- Iterates through the parameter versions and formats the output to display the version name and state (enabled/disabled).
- Adds a new function
- parametermanager/regional_samples/regional_parametermanager_test.go
- Adds new tests for
getRegionalParam,getRegionalParamVersion,renderRegionalParamVersion,listRegionalParam, andlistRegionalParamVersion. - Includes helper functions for creating and cleaning up parameters, parameter versions, and secrets.
- Uses
testutil.SystemTestfor integration testing with a GCP project. - Validates the output of the samples against expected values.
- Adds new tests for
- parametermanager/regional_samples/render_regional_param_version.go
- Adds a new function
renderRegionalParamVersionto render a regional parameter version. - Includes code to create a Parameter Manager client with a regional endpoint.
- Formats the output to display the rendered parameter version and payload.
- Adds a new function
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A parameter's tale,
Versions listed, without fail,
Rendered secrets gleam,
In GCP's vast stream,
A cloud function's trail.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request adds samples for viewing, listing, and rendering parameters and parameter versions using the Parameter Manager SDK. The code includes the necessary functions and tests for these operations. Overall, the code is well-structured and follows Go best practices. However, there are a few areas that could be improved for clarity and efficiency.
Summary of Findings
- Error Handling in Tests: The test functions use
t.Fatalffor error handling, which immediately stops the test. Consider usingt.Errorfto allow the test to continue and report multiple errors. - Redundant Location ID: The
locationIdvariable is defined and used in multiple test functions. Consider defining it once at the package level to avoid redundancy. - Missing Error Check: The
testIamGrantAccessfunction does not include error handling for theclient.IAMcall, which could lead to unexpected behavior if the IAM client fails to initialize.
Merge Readiness
The pull request is almost ready for merging. Addressing the error handling in tests and the redundant location ID would improve the code quality. The missing error check in testIamGrantAccess should be addressed before merging. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| } | ||
| defer client.Close() | ||
|
|
||
| handle := client.IAM(name) |
There was a problem hiding this comment.
Consider adding error handling for the client.IAM call. If the IAM client fails to initialize, it could lead to unexpected behavior.
handle := client.IAM(name)
if handle == nil {
return fmt.Errorf("failed to create IAM handle")
}| endpoint := fmt.Sprintf("secretmanager.%s.rep.googleapis.com:443", locationId) | ||
| client, err := secretmanager.NewClient(ctx, option.WithEndpoint(endpoint)) | ||
| if err != nil { | ||
| t.Fatalf("failed to create client: %v", err) |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
| t.Fatalf("failed to create client: %v", err) | |
| t.Errorf("failed to create client: %v", err) |
|
|
||
| endpoint := fmt.Sprintf("secretmanager.%s.rep.googleapis.com:443", locationId) | ||
| client, err := secretmanager.NewClient(ctx, option.WithEndpoint(endpoint)) | ||
| if err != nil { |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
t.Errorf("failed to create client: %v", err)| }, | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("testParameter: failed to create parameter: %v", err) |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
| t.Fatalf("testParameter: failed to create parameter: %v", err) | |
| t.Errorf("testParameter: failed to create parameter: %v", err) |
|
|
||
| ctx := context.Background() | ||
| endpoint := fmt.Sprintf("parametermanager.%s.rep.googleapis.com:443", locationId) | ||
| client, err := parametermanager.NewClient(ctx, option.WithEndpoint(endpoint)) |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
| client, err := parametermanager.NewClient(ctx, option.WithEndpoint(endpoint)) | |
| t.Errorf("failed to create client: %v", err) |
| locationId := testLocation(t) | ||
| var b bytes.Buffer | ||
| if err := getRegionalParam(&b, tc.ProjectID, locationId, parameterID); err != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
t.Errorf("GetParameter: %v", err)| locationId := testLocation(t) | ||
|
|
||
| var b bytes.Buffer | ||
| if err := getRegionalParamVersion(&b, tc.ProjectID, locationId, parameterID, parameterVersionID); err != nil { |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
t.Errorf("GetParameterVersion: %v", err)|
|
||
| var b bytes.Buffer | ||
| time.Sleep(2 * time.Minute) | ||
| if err := renderRegionalParamVersion(&b, tc.ProjectID, locationId, parameterID, parameterVersionID); err != nil { |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
t.Errorf("RenderParameterVersion: %v", err)|
|
||
| var b bytes.Buffer | ||
| if err := listRegionalParam(&b, tc.ProjectID, locationId); err != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
t.Errorf("ListParameter: %v", err)| defer testCleanupParameterVersion(t, parameterVersion2.Name) | ||
|
|
||
| var b bytes.Buffer | ||
| if err := listRegionalParamVersion(&b, tc.ProjectID, locationId, parameterID); err != nil { |
There was a problem hiding this comment.
Using t.Fatalf will halt the test immediately upon failure. For better reporting, consider using t.Errorf to allow the test to continue and report multiple failures.
t.Errorf("ListParameterVersion: %v", err)
arpangoswami
added
kokoro:force-run
kokoro-team
removed
kokoro:run
Head branch was pushed to by a user without write access
| t.Fatal(err) | ||
| } | ||
|
|
||
| if got, want := b.String(), fmt.Sprintf("Found parameter %s with format %s \n", parameter1.Name, parameter1.Format); !strings.Contains(got, want) { | ||
| if got, want := buf.String(), fmt.Sprintf("Found parameter %s with format %s \n", parameter1.Name, parameter1.Format); !strings.Contains(got, want) { |
There was a problem hiding this comment.
issue(non-blocking): for reducing the number of chars in the line, the want message content could be reduced to: "Found parameter %s with format %s"
There was a problem hiding this comment.
I've updated the want message.
| t.Errorf("ListParameter: expected %q to contain %q", got, want) | ||
| } | ||
|
|
||
| if got, want := b.String(), fmt.Sprintf("Found parameter %s with format %s \n", parameter2.Name, parameter2.Format); !strings.Contains(got, want) { | ||
| if got, want := buf.String(), fmt.Sprintf("Found parameter %s with format %s \n", parameter2.Name, parameter2.Format); !strings.Contains(got, want) { |
There was a problem hiding this comment.
issue(non-blocking): for reducing the number of chars in the line, the want message content could be reduced to: "Found parameter %s with format %s"
There was a problem hiding this comment.
I've updated the want message.
parametermanager/list_param.go
Outdated
| if err == iterator.Done { | ||
| break | ||
| } | ||
| if err != nil { | ||
| return fmt.Errorf("failed to list parameters: %w", err) | ||
| } | ||
|
|
||
| fmt.Fprintf(w, "Found parameter %s with format %s \n", resp.Name, resp.Format.String()) | ||
| fmt.Fprintf(w, "Found parameter %s with format %s \n", parameter.Name, parameter.Format.String()) |
There was a problem hiding this comment.
issue(non-blocking): for reducing the number of chars in the line, the want message content could be reduced to: "Found parameter %s with format %s"
There was a problem hiding this comment.
I've updated the want message.
OremGLG
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
|
Please, fix your PR conflicts in order to re-run the checks. |
@OremGLG Resolved the merge conflict. |
|
Still with merge conflicts 😬 |
One of the PR got merged, which caused the conflict — I’ve resolved. |
Description
Added samples for view, list and render parameter and parameter versions using Parameter Manager SDK
Sample List (regional):
Also added required Tests for the same.
Checklist
go test -v ./..(see Testing)gofmt(see Formatting)go vet(see Formatting)GOLANG_REGIONAL_SAMPLES_LOCATIONvariable to be set