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

Add a show command to the Diki CLI #412

Merged
merged 27 commits into from
Jan 22, 2025

Conversation

georgibaltiev
Copy link
Contributor

What this PR does / why we need it:
This PR implements a new Cobra command that generates a JSON object containing the metadata of the specified provider and it's supported rulesets, or a JSON list of all providers available.

Which issue(s) this PR fixes:
Fixes #300

Special notes for your reviewer:

Release note:

Diki now has a `show` command that reveals metadata about the binary's supported providers and their rulesets

@georgibaltiev georgibaltiev requested a review from a team as a code owner December 23, 2024 11:55
@gardener-robot gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Dec 23, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 23, 2024
@gardener-robot gardener-robot added size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else and removed size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Dec 27, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 27, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 27, 2024
Copy link
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

Thanks! I did an initial review. I would probably go once again over the PR once the comments are addressed.

Comment on lines 142 to 143
Short: "",
Long: "",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Short: "",
Long: "",
Short: "Show detailed information for the given provider.",
Long: "Show detailed information for the given provider.",

@@ -0,0 +1,41 @@
// SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and Gardener contributors
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and Gardener contributors
// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors


// Version is used to represent a specific version of a ruleset
type Version struct {
// Version is the human-readable name of the ruleset release
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Version is the human-readable name of the ruleset release
// Version is the name of the ruleset release.

type Version struct {
// Version is the human-readable name of the ruleset release
Version string `json:"version"`
// Latest is a bool tag that showcases if the specific version is the latest one
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Latest is a bool tag that showcases if the specific version is the latest one
// Latest shows if the specific version is the latest one.

ProviderName string `json:"name"`
}

// ProviderMetadata is used to represent a specific provider and it's metadata
Copy link
Member

Choose a reason for hiding this comment

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

Please put dots at the end of all sentences.

// GardenProviderMetadata returns available metadata for the Garden Provider and it's supported rulesets.
func GardenProviderMetadata() metadata.ProviderMetadata {
providerMetadata := metadata.ProviderMetadata{}
providerMetadata.ProviderID = "garden"
Copy link
Member

Choose a reason for hiding this comment

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

This would not be correct if the provider id changes here

diki/cmd/diki/main.go

Lines 19 to 22 in a4a8e8f

"garden": builder.GardenProviderFromConfig,
"gardener": builder.GardenerProviderFromConfig,
"managedk8s": builder.ManagedK8SProviderFromConfig,
"virtualgarden": builder.VirtualGardenProviderFromConfig,

We should either fix the provider id to a constant or not hardcode it here.

Comment on lines 69 to 89
var availableRulesets = map[string]string{
securityhardenedshoot.RulesetID: securityhardenedshoot.RulesetName,
}

for rulesetID, rulesetName := range availableRulesets {
rulesetMetadata := &metadata.RulesetMetadata{}
rulesetMetadata.RulesetID = rulesetID
rulesetMetadata.RulesetName = rulesetName
rulesetSupportedVersions := gardenGetSupportedVersions(rulesetMetadata.RulesetID)

for index, supportedVersion := range rulesetSupportedVersions {
if index == 0 {
rulesetMetadata.Versions = append(rulesetMetadata.Versions, metadata.Version{Version: supportedVersion, Latest: true})
} else {
rulesetMetadata.Versions = append(rulesetMetadata.Versions, metadata.Version{Version: supportedVersion, Latest: false})
}
}
providerMetadata.ProviderRulesets = append(providerMetadata.ProviderRulesets, *rulesetMetadata)
}

return providerMetadata
Copy link
Member

Choose a reason for hiding this comment

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

Why not just initialize the structs?

providerMetadata := metadata.ProviderMetadata{
		ProviderID:   "garden",
		ProviderName: "Garden",
		ProviderRulesets: []metadata.RulesetMetadata{
			{
				RulesetID:   securityhardenedshoot.RulesetID,
				RulesetName: securityhardenedshoot.RulesetName,
			},
		},
	}

	for i := range providerMetadata.ProviderRulesets {
		supportedVersions := gardenGetSupportedVersions(providerMetadata.ProviderRulesets[i].RulesetID)
		for _, supportedVersion := range supportedVersions {
			providerMetadata.ProviderRulesets[i].Versions = append(
				providerMetadata.ProviderRulesets[i].Versions,
				metadata.Version{Version: supportedVersion, Latest: false},
			)
		}

		// Mark the first version as latest as the versions are sorted from newest to oldest
		if len(providerMetadata.ProviderRulesets[i].Versions) > 0 {
			providerMetadata.ProviderRulesets[i].Versions[0].Latest = true
		}
	}

	return providerMetadata

cmd/diki/app/app.go Show resolved Hide resolved
Comment on lines 209 to 224
} else {
var providerArg = args[0]

metadataFunc, ok := providerFuncMap[providerArg]
if !ok {
return fmt.Errorf("provider %s does not exist in the current diki binary", providerArg)
}

providerMetadata := metadataFunc()

if bytes, err := json.Marshal(providerMetadata); err != nil {
return err
} else {
fmt.Println(string(bytes))
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} else {
var providerArg = args[0]
metadataFunc, ok := providerFuncMap[providerArg]
if !ok {
return fmt.Errorf("provider %s does not exist in the current diki binary", providerArg)
}
providerMetadata := metadataFunc()
if bytes, err := json.Marshal(providerMetadata); err != nil {
return err
} else {
fmt.Println(string(bytes))
}
}
}
metadataFunc, ok := providerFuncMap[args[0]]
if !ok {
return fmt.Errorf("provider %s does not exist in the current diki binary", args[0])
}
if bytes, err := json.Marshal(metadataFunc()); err != nil {
return err
}
fmt.Println(string(bytes))

@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 7, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 7, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 7, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 7, 2025
Copy link
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

Just a few minor remarks

@@ -124,6 +125,28 @@ e.g. to check compliance of your hyperscaler accounts.`,
addReportGenerateDiffFlags(generateDiffCmd, &generateDiffOpts)
generateCmd.AddCommand(generateDiffCmd)

showCmd := &cobra.Command{
Use: "show",
Short: "Show metadata of the providers that the current diki binary supports.",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Short: "Show metadata of the providers that the current diki binary supports.",
Short: "Show metadata information for different diki internals, i.e. providers.",


showProviderCmd := &cobra.Command{
Use: "provider",
Short: "Show detailed information for the given provider.",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Short: "Show detailed information for the given provider.",
Short: "Show detailed information for providers.",


metadataFunc, ok := metadataFuncs[args[0]]
if !ok {
return fmt.Errorf("provider %s does not exist in the current diki binary", args[0])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("provider %s does not exist in the current diki binary", args[0])
return fmt.Errorf("unknown provider: %s", args[0])

"github.com/gardener/diki/pkg/provider"
"github.com/gardener/diki/pkg/report"
"github.com/gardener/diki/pkg/ruleset"
)

// NewDikiCommand creates a new command that is used to start Diki.
func NewDikiCommand(providerCreateFuncs map[string]provider.ProviderFromConfigFunc) *cobra.Command {
func NewDikiCommand(providerCreateFuncs map[string]provider.ProviderFromConfigFunc, metadataFuncs map[string]metadata.MetadataFunc) *cobra.Command {
Copy link
Member

Choose a reason for hiding this comment

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

Use a single map, so there is consistency across provider id keys.

@@ -156,6 +179,39 @@ func addReportGenerateDiffFlags(cmd *cobra.Command, opts *generateDiffOptions) {
cmd.PersistentFlags().Var(cliflag.NewMapStringString(&opts.identityAttributes), "identity-attributes", "The keys are the IDs of the providers that will be present in the generated difference report and the values are metadata attributes to be used as identifiers.")
}

func showProviderCmd(args []string, metadataFuncs map[string]metadata.MetadataFunc) error {
if len(args) > 1 {
return errors.New("command `show provider` accepts at most one provider")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return errors.New("command `show provider` accepts at most one provider")
return errors.New("command 'show provider' accepts at most one provider")

}

// MetadataFunc constructs a detailed Provider metadata object.
type MetadataFunc func() ProviderDetailed
Copy link
Member

Choose a reason for hiding this comment

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

Please move this func to pkg/provider/provider.go

pkg/provider/virtualgarden/provider.go Show resolved Hide resolved
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 21, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 21, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 21, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 21, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 21, 2025
Copy link
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review needs/second-opinion Needs second review by someone else labels Jan 22, 2025
@dimityrmirchev dimityrmirchev merged commit 6e41ef6 into gardener:main Jan 22, 2025
9 checks passed
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 22, 2025
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/l Size of pull request is large (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add show command
6 participants