feat(security-command-center): add v2 version of SetMuteUndefinedFinding.java #9589
Conversation
product-auto-label
bot
added
samples
|
Blocked until #9580 is merged. |
|
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
That PR was merged, and this one is ready to review. |
|
@telpirion Friendly ping. |
| * limitations under the License. | ||
| */ | ||
|
|
||
| package vtwo.muteconfig; |
There was a problem hiding this comment.
issue: rename to v2.muteconfig.
question: I see that there is a muteconfig package already in the parent java/ folder. How do we plan to disambiguate between the two packages?
There was a problem hiding this comment.
Every existing file under security-command-center/snippets/src/main/java/vtwo follows the naming convention that I've used here. The existing files all use a package name that starts with vtwo, not v2. Also, the existing files often have package names that overlap with the parent java/ folder.
Can I ask that you allow me to add this sample with the package name vtwo.muteconfig, and that you work with the sample owners (the Security Command Center product team) to improve the package names as a follow-on task?
@owenhuyn FYI.
| // Reset mute state of an individual finding. | ||
| // If a finding is already reset, resetting it again has no effect. | ||
| // Various mute states are: MUTE_UNSPECIFIED/MUTE/UNMUTE/UNDEFINED. | ||
| public static Finding setMuteUndefined(String findingPath) throws IOException { |
There was a problem hiding this comment.
issue: don't return an object, instead process the result in the sample.
See https://googlecloudplatform.github.io/samples-style-guide/#result.
There was a problem hiding this comment.
In this repo's SAMPLE_FORMAT.md file, it says the opposite:
Snippet methods should return data that can be used in the calling method to show the user how to interact with a returned object programmatically.
In #9547, I tried to log information instead of returning a result, as you're suggesting, and I was told to return a result instead.
I'd like to keep this code as-is, given that it's consistent with the instructions in this repo.
| Finding finding = SetMuteFinding.setMute(FINDING_1.getName()); | ||
| assertThat(finding.getMute()).isEqualTo(Mute.MUTED); | ||
| finding = SetUnmuteFinding.setUnmute(FINDING_1.getName()); | ||
| assertThat(finding.getMute()).isEqualTo(Mute.UNMUTED); | ||
| finding = SetMuteUndefinedFinding.setMuteUndefined(FINDING_1.getName()); |
There was a problem hiding this comment.
issue: don't check for a result; instead parse stdout to ensure that something was printed.
There was a problem hiding this comment.
I'd like to keep this code as-is, given that it's consistent with the instructions in this repo.
Description
Adds a v2 equivalent of
security-command-center/snippets/src/main/java/muteconfig/SetMuteUndefinedFinding.java.Checklist
pom.xmlparent set to latestshared-configurationmvn clean verifyrequiredmvn -P lint checkstyle:checkrequiredmvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory only