diff --git a/docs/cli/konstraint.md b/docs/cli/konstraint.md index b4e6dae6..5bb94739 100644 --- a/docs/cli/konstraint.md +++ b/docs/cli/konstraint.md @@ -14,7 +14,6 @@ A tool to create and manage Gatekeeper CRDs from Rego ### SEE ALSO -* [konstraint convert](konstraint_convert.md) - Convert legacy annotations to OPA Metadata Annotations * [konstraint create](konstraint_create.md) - Create Gatekeeper constraints from Rego policies * [konstraint doc](konstraint_doc.md) - Generate documentation from Rego policies diff --git a/docs/cli/konstraint_create.md b/docs/cli/konstraint_create.md index 000ab8cf..43318e47 100644 --- a/docs/cli/konstraint_create.md +++ b/docs/cli/konstraint_create.md @@ -23,7 +23,7 @@ Create constraints with the Gatekeeper enforcement action set to dryrun ``` --constraint-template-version string Set the version of ConstraintTemplates (default "v1beta1") - -d, --dryrun Sets the enforcement action of the constraints to dryrun, overriding the @enforcement tag + -d, --dryrun Sets the enforcement action of the constraints to dryrun, overriding the enforcement setting -h, --help help for create -o, --output string Specify an output directory for the Gatekeeper resources --partial-constraints Generate partial Constraints for policies with parameters diff --git a/docs/constraint_creation.md b/docs/constraint_creation.md index dbf8712c..c969e3ee 100644 --- a/docs/constraint_creation.md +++ b/docs/constraint_creation.md @@ -112,9 +112,7 @@ in the custom metadata section. ### Legacy annotations -Previously Konstraint had custom annotation format, such as `@title` or `@kinds`, which is a legacy format and will be removed in future releases. - -To aid with transition to OPA Metadata format, a conversion tool is provided as `konstraint convert` +Previously Konstraint had custom annotation format, such as `@title` or `@kinds`, which is a legacy format and were removed in release v0.39.0. ## Using Input Parameters diff --git a/internal/commands/convert.go b/internal/commands/convert.go deleted file mode 100644 index a066a5e2..00000000 --- a/internal/commands/convert.go +++ /dev/null @@ -1,123 +0,0 @@ -package commands - -import ( - "fmt" - "os" - "strings" - - "github.com/plexsystems/konstraint/internal/rego" - log "github.com/sirupsen/logrus" - "github.com/spf13/cobra" - "sigs.k8s.io/yaml" -) - -func newConvertCommand() *cobra.Command { - cmd := cobra.Command{ - Use: "convert ", - Short: "Convert legacy annotations to OPA Metadata Annotations", - Long: "Converts legacy annotations to OPA Metadata Annotations. Policies that already have any package-level OPA Metadata Annotations set will be skipped, even if the policy contains legacy annotations.", - Example: `Convert all policies with legacy annotations to OPA Metadata annotations in-place - -konstraint convert examples`, - - RunE: func(_ *cobra.Command, args []string) error { - path := "." - if len(args) > 0 { - path = args[0] - } - - return runConvertCommand(path) - }, - } - - return &cmd -} - -func runConvertCommand(path string) error { - regos, err := rego.GetAllSeveritiesWithoutImports(path) - if err != nil { - return fmt.Errorf("get regos: %w", err) - } - - var conversions int - - for _, r := range regos { - logger := log.WithFields(log.Fields{ - "name": r.Kind(), - "src": r.Path(), - }) - - if r.HasMetadataAnnotations() { - logger.Info("was skipped since it has OPA Annotations already") - continue - } - - conveted, err := r.ConvertLegacyAnnotations() - if err != nil { - logger.WithError(err).Error("Failed to convert legacy annotations") - } - - var sb strings.Builder - sb.WriteString("# METADATA\n") - - // Force order: `title`, `description`, `custom` - if conveted.Title != "" { - yml, err := yaml.Marshal(®o.ConvertedLegacyAnnotations{Title: conveted.Title}) - if err != nil { - logger.WithError(err).Error("Failed to marshal OPA Annotations title field to YAML") - continue - } - appendCommentedYaml(&sb, yml) - } - if conveted.Description != "" { - yml, err := yaml.Marshal(®o.ConvertedLegacyAnnotations{Description: conveted.Description}) - if err != nil { - logger.WithError(err).Error("Failed to marshal OPA Annotations description field to YAML") - continue - } - appendCommentedYaml(&sb, yml) - } - if len(conveted.Custom) > 0 { - yml, err := yaml.Marshal(®o.ConvertedLegacyAnnotations{Custom: conveted.Custom}) - if err != nil { - logger.WithError(err).Error("Failed to marshal OPA Annotations custom field to YAML") - continue - } - appendCommentedYaml(&sb, yml) - } - - sb.WriteString(r.LegacyConversionSource()) - sb.WriteByte('\n') - - if err := os.WriteFile(r.Path(), []byte(sb.String()), 0644); err != nil { - return fmt.Errorf("writing updated policy source: %w", err) - } - - conversions++ - logger.Info("converted successfully") - } - - log.WithFields(log.Fields{ - "num_policies": len(regos), - "num_converted": conversions, - }).Info("Completed successfully.") - - return nil -} - -func appendCommentedYaml(sb *strings.Builder, yaml []byte) { - split := strings.Split(string(yaml), "\n") - - for i, line := range split { - // avoid trailing spaces - if line == "" { - // skip last line if it's empty - if i != len(split)-1 { - sb.WriteString("#\n") - } - continue - } - - sb.WriteString("# " + line + "\n") - } -} diff --git a/internal/commands/create.go b/internal/commands/create.go index 91d16ced..030508e4 100644 --- a/internal/commands/create.go +++ b/internal/commands/create.go @@ -1,7 +1,6 @@ package commands import ( - "encoding/json" "fmt" "os" "path/filepath" @@ -20,10 +19,6 @@ import ( "sigs.k8s.io/yaml" ) -const ( - legacyMigrationMessage = " are set with legacy annotations, this functionality will be removed in a future release. Please migrate to OPA Metadata annotations. See konstraint convert." -) - func newCreateCommand() *cobra.Command { cmd := cobra.Command{ Use: "create ", @@ -64,7 +59,7 @@ Create constraints with the Gatekeeper enforcement action set to dryrun } cmd.PersistentFlags().StringP("output", "o", "", "Specify an output directory for the Gatekeeper resources") - cmd.PersistentFlags().BoolP("dryrun", "d", false, "Sets the enforcement action of the constraints to dryrun, overriding the @enforcement tag") + cmd.PersistentFlags().BoolP("dryrun", "d", false, "Sets the enforcement action of the constraints to dryrun, overriding the enforcement setting") cmd.PersistentFlags().Bool("skip-constraints", false, "Skip generation of constraints") cmd.PersistentFlags().String("constraint-template-version", "v1beta1", "Set the version of ConstraintTemplates") cmd.PersistentFlags().Bool("partial-constraints", false, "Generate partial Constraints for policies with parameters") @@ -132,7 +127,7 @@ func runCreateCommand(path string) error { } // Skip Constraint generation if there are parameters on the template. - if !viper.GetBool("partial-constraints") && (len(violation.Parameters()) > 0 || len(violation.AnnotationParameters()) > 0) { + if !viper.GetBool("partial-constraints") && len(violation.AnnotationParameters()) > 0 { logger.Warn("Skipping constraint generation due to use of parameters") continue } @@ -157,7 +152,7 @@ func runCreateCommand(path string) error { return nil } -func getConstraintTemplatev1(violation rego.Rego, logger *log.Entry) *v1.ConstraintTemplate { +func getConstraintTemplatev1(violation rego.Rego, _ *log.Entry) *v1.ConstraintTemplate { constraintTemplate := v1.ConstraintTemplate{ TypeMeta: metav1.TypeMeta{ APIVersion: "templates.gatekeeper.sh/v1", @@ -184,20 +179,7 @@ func getConstraintTemplatev1(violation rego.Rego, logger *log.Entry) *v1.Constra }, } - if len(violation.Parameters()) > 0 { - logger.Warn("Parameters" + legacyMigrationMessage) - constraintTemplate.Spec.CRD.Spec.Validation = &v1.Validation{ - OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ - Properties: violation.GetOpenAPISchemaProperties(), - Type: "object", - }, - } - } - if len(violation.AnnotationParameters()) > 0 { - if constraintTemplate.Spec.CRD.Spec.Validation != nil { - logger.Warn("Parameters already set with legacy annotations, overwriting the parameters using values from OPA Metadata") - } constraintTemplate.Spec.CRD.Spec.Validation = &v1.Validation{ OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ Properties: violation.AnnotationParameters(), @@ -209,7 +191,7 @@ func getConstraintTemplatev1(violation rego.Rego, logger *log.Entry) *v1.Constra return &constraintTemplate } -func getConstraintTemplatev1beta1(violation rego.Rego, logger *log.Entry) *v1beta1.ConstraintTemplate { +func getConstraintTemplatev1beta1(violation rego.Rego, _ *log.Entry) *v1beta1.ConstraintTemplate { constraintTemplate := v1beta1.ConstraintTemplate{ TypeMeta: metav1.TypeMeta{ APIVersion: "templates.gatekeeper.sh/v1beta1", @@ -236,19 +218,7 @@ func getConstraintTemplatev1beta1(violation rego.Rego, logger *log.Entry) *v1bet }, } - if len(violation.Parameters()) > 0 { - logger.Warn("Parameters" + legacyMigrationMessage) - constraintTemplate.Spec.CRD.Spec.Validation = &v1beta1.Validation{ - OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ - Properties: violation.GetOpenAPISchemaProperties(), - }, - } - } - if len(violation.AnnotationParameters()) > 0 { - if constraintTemplate.Spec.CRD.Spec.Validation != nil { - logger.Warn("Parameters already set with legacy annotations, overwriting the parameters using values from OPA Metadata") - } constraintTemplate.Spec.CRD.Spec.Validation = &v1beta1.Validation{ OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ Properties: violation.AnnotationParameters(), @@ -259,7 +229,7 @@ func getConstraintTemplatev1beta1(violation rego.Rego, logger *log.Entry) *v1bet return &constraintTemplate } -func getConstraint(violation rego.Rego, logger *log.Entry) (*unstructured.Unstructured, error) { +func getConstraint(violation rego.Rego, _ *log.Entry) (*unstructured.Unstructured, error) { gvk := schema.GroupVersionKind{ Group: "constraints.gatekeeper.sh", Version: "v1beta1", @@ -292,68 +262,14 @@ func getConstraint(violation rego.Rego, logger *log.Entry) (*unstructured.Unstru } } - matchers, err := violation.Matchers() - if err != nil { - return nil, fmt.Errorf("get matchers: %w", err) - } - - if len(matchers.KindMatchers) > 0 { - logger.Warn("Kind Matchers" + legacyMigrationMessage) - if err := setKindMatcher(&constraint, matchers.KindMatchers); err != nil { - return nil, fmt.Errorf("set kind matcher: %w", err) - } - } - - if len(matchers.MatchLabelsMatcher) > 0 { - logger.Warn("Match Labels Matchers" + legacyMigrationMessage) - if err := setMatchLabelsMatcher(&constraint, matchers.MatchLabelsMatcher); err != nil { - return nil, fmt.Errorf("set match labels matcher: %w", err) - } - } - - if len(matchers.MatchExpressionsMatcher) > 0 { - logger.Warn("Match Expressions Matchers" + legacyMigrationMessage) - if err := setMatchExpressionsMatcher(&constraint, matchers.MatchExpressionsMatcher); err != nil { - return nil, fmt.Errorf("set match expressions matcher: %w", err) - } - } - - if len(matchers.NamespaceMatcher) > 0 { - logger.Warn("Namespace Matchers" + legacyMigrationMessage) - if err := setNestedStringSlice(&constraint, matchers.NamespaceMatcher, "spec", "match", "namespaces"); err != nil { - return nil, fmt.Errorf("set namespace matcher: %w", err) - } - } - - if len(matchers.ExcludedNamespaceMatcher) > 0 { - logger.Warn("Excluded Namespace Matchers" + legacyMigrationMessage) - if err := setNestedStringSlice(&constraint, matchers.ExcludedNamespaceMatcher, "spec", "match", "excludedNamespaces"); err != nil { - return nil, fmt.Errorf("set namespace matcher: %w", err) - } - } - metadataMatchers, ok := violation.GetAnnotation("matchers") if ok { - if len(matchers.KindMatchers) > 0 || - len(matchers.MatchLabelsMatcher) > 0 || - len(matchers.MatchExpressionsMatcher) > 0 || - len(matchers.NamespaceMatcher) > 0 || - len(matchers.ExcludedNamespaceMatcher) > 0 { - logger.Warn("Overwriting matchers set with legacy annotations using matchers from OPA Metadata.") - } - if err := unstructured.SetNestedField(constraint.Object, metadataMatchers, "spec", "match"); err != nil { return nil, fmt.Errorf("set matchers from metadata annotation: %w", err) } } if viper.GetBool("partial-constraints") { - if len(violation.Parameters()) > 0 { - logger.Warn("Parameters" + legacyMigrationMessage) - if err := addParametersToConstraintLegacy(&constraint, violation.Parameters()); err != nil { - return nil, fmt.Errorf("add parameters %v to constraint: %w", violation.Parameters(), err) - } - } if len(violation.AnnotationParameters()) > 0 { if err := addParametersToConstraint(&constraint, violation.AnnotationParameters()); err != nil { return nil, fmt.Errorf("add parameters %v to constraint: %w", violation.AnnotationParameters(), err) @@ -376,52 +292,6 @@ func addParametersToConstraint(constraint *unstructured.Unstructured, parameters return nil } -func addParametersToConstraintLegacy(constraint *unstructured.Unstructured, parameters []rego.Parameter) error { - params := make(map[string]interface{}, len(parameters)) - for _, p := range parameters { - params[p.Name] = nil - } - if err := unstructured.SetNestedField(constraint.Object, params, "spec", "parameters"); err != nil { - return fmt.Errorf("set parameters map: %w", err) - } - - return nil -} - -func setKindMatcher(constraint *unstructured.Unstructured, kindMatchers rego.KindMatchers) error { - if err := unstructured.SetNestedSlice(constraint.Object, kindMatchers.ToSpec(), "spec", "match", "kinds"); err != nil { - return fmt.Errorf("set constraint kinds matchers: %w", err) - } - return nil -} - -func setMatchLabelsMatcher(constraint *unstructured.Unstructured, matcher rego.MatchLabelsMatcher) error { - if err := unstructured.SetNestedStringMap(constraint.Object, matcher, "spec", "match", "labelSelector", "matchLabels"); err != nil { - return fmt.Errorf("set constraint labelSelector.matchLabels matchers: %w", err) - } - return nil -} - -func setMatchExpressionsMatcher(constraint *unstructured.Unstructured, matcher []rego.MatchExpressionMatcher) error { - marshaled, err := json.Marshal(matcher) - if err != nil { - return err - } - var unmarshaled []interface{} - if err := json.Unmarshal(marshaled, &unmarshaled); err != nil { - return err - } - return unstructured.SetNestedSlice(constraint.Object, unmarshaled, "spec", "match", "labelSelector", "matchExpressions") -} - -func setNestedStringSlice(constraint *unstructured.Unstructured, slice []string, path ...string) error { - var values []interface{} - for _, s := range slice { - values = append(values, interface{}(s)) - } - return unstructured.SetNestedSlice(constraint.Object, values, path...) -} - func isValidEnforcementAction(action string) bool { for _, a := range []string{"deny", "dryrun", "warn"} { if a == action { diff --git a/internal/commands/default.go b/internal/commands/default.go index 415beb95..37e4e136 100644 --- a/internal/commands/default.go +++ b/internal/commands/default.go @@ -24,7 +24,6 @@ func NewDefaultCommand() *cobra.Command { cmd.AddCommand(newCreateCommand()) cmd.AddCommand(newDocCommand()) - cmd.AddCommand(newConvertCommand()) return &cmd } diff --git a/internal/commands/document.go b/internal/commands/document.go index f76b01e5..75ee9033 100644 --- a/internal/commands/document.go +++ b/internal/commands/document.go @@ -188,21 +188,12 @@ func getDocumentation(path string, outputDirectory string) (map[rego.Severity][] anchor := strings.ToLower(strings.ReplaceAll(documentTitle, " ", "-")) anchor = strings.ReplaceAll(anchor, ":", "") - legacyMatchers, err := policy.Matchers() - if err != nil { - return nil, fmt.Errorf("parse matchers from legacy annotations: %w", err) - } - var matchResources []string if len(policy.AnnotationKindMatchers()) > 0 { for _, akm := range policy.AnnotationKindMatchers() { s := strings.Split(akm.String(), " ") matchResources = append(matchResources, s...) } - } else { - if len(legacyMatchers.KindMatchers) > 0 { - matchResources = strings.Split(legacyMatchers.KindMatchers.String(), " ") - } } if len(matchResources) == 0 { logger.Warn("No kind matchers set, this can lead to poor policy performance.") @@ -212,14 +203,9 @@ func getDocumentation(path string, outputDirectory string) (map[rego.Severity][] var matchLabels string if policy.AnnotationLabelSelectorMatcher() != nil { matchLabels = labelSelectorDocString(policy.AnnotationLabelSelectorMatcher()) - } else { - matchLabels = legacyMatchers.MatchLabelsMatcher.String() } - parameters := policy.Parameters() - if len(policy.AnnotationParameters()) > 0 { - parameters = annoParamsToLegacyFormat(policy.AnnotationParameters()) - } + parameters := annoParamsToLegacyFormat(policy.AnnotationParameters()) sort.Slice(parameters, func(i, j int) bool { return parameters[i].Name < parameters[j].Name }) diff --git a/internal/rego/matchers.go b/internal/rego/matchers.go deleted file mode 100644 index 191bab86..00000000 --- a/internal/rego/matchers.go +++ /dev/null @@ -1,248 +0,0 @@ -package rego - -import ( - "fmt" - "sort" - "strings" -) - -// Matchers are all of the matchers that can be applied to constraints. -type Matchers struct { - KindMatchers KindMatchers - MatchLabelsMatcher MatchLabelsMatcher - MatchExpressionsMatcher []MatchExpressionMatcher - NamespaceMatcher []string - ExcludedNamespaceMatcher []string -} - -// kindMatchersMap is internal representation of KindMatchers -// it maps apiGroup to a slice of kinds -type kindMatchersMap map[string][]string - -// KindMatcher is the matcher to generate `constraints.spec.match.kinds` -type KindMatcher struct { - APIGroup string - Kinds []string -} - -const ( - coreAPIGroup = "core" - coreAPIShorthand = "" -) - -// KindMatchers is a slice of KindMatcher -type KindMatchers []KindMatcher - -func (k KindMatchers) String() string { - var result string - for _, kindMatcher := range k { - apiGroup := kindMatcher.APIGroup - if apiGroup == coreAPIShorthand { - apiGroup = coreAPIGroup - } - for _, kind := range kindMatcher.Kinds { - result += apiGroup + "/" + kind + " " - } - } - - result = strings.TrimSpace(result) - return result -} - -// ToSpec converts KindMatchers to a slice in format -// compatible with `spec.match.kinds` of a Constraint -func (k KindMatchers) ToSpec() []any { - specSlice := make([]any, len(k)) - - for i, matcher := range k { - // convert to interface slice so apimachinery's runtime.DeepCopy works - kinds := make([]any, len(matcher.Kinds)) - for j, kind := range matcher.Kinds { - kinds[j] = any(kind) - } - - specSlice[i] = map[string]any{ - "apiGroups": []any{any(matcher.APIGroup)}, - "kinds": kinds, - } - } - - return specSlice -} - -// addIfNotPresent adds an apiGroup/kind matcher to the map -// unless it's already present -// -// it also transforms apiGroup `"core"` to `""` -func (k kindMatchersMap) addIfNotPreset(apiGroup, kind string) { - if apiGroup == coreAPIGroup { - apiGroup = coreAPIShorthand - } - for _, item := range k[apiGroup] { - if strings.EqualFold(kind, item) { - return - } - } - k[apiGroup] = append(k[apiGroup], kind) -} - -// convert converts kindMatchersMap to KindMatchers, -// sorted by apiGroup, with kinds in each apiGroupKinds sorted -func (k kindMatchersMap) convert() KindMatchers { - apiGroups := make([]string, 0, len(k)) - for apiGroup := range k { - apiGroups = append(apiGroups, apiGroup) - } - sort.Strings(apiGroups) - - result := make(KindMatchers, len(apiGroups)) - for i, apiGroup := range apiGroups { - result[i].APIGroup = apiGroup - kinds := k[apiGroup] - sort.Strings(kinds) - result[i].Kinds = kinds - } - - return result -} - -// MatchLabelsMatcher is the matcher to generate `constraints.spec.match.labelSelector.matchLabels`. -type MatchLabelsMatcher map[string]string - -// MatchExpressionsMatcher is the matcher to generate `constraints.spec.match.labelSelector.matchExpressions`. -type MatchExpressionMatcher struct { - Key string `json:"key"` - Operator string `json:"operator"` - Values []string `json:"values,omitempty"` -} - -func (m MatchLabelsMatcher) String() string { - var result string - for k, v := range m { - result += fmt.Sprintf("%s=%s ", k, v) - } - - return strings.TrimSpace(result) -} - -// Matchers returns all of the matchers found in the rego file. -func (r Rego) Matchers() (Matchers, error) { - matchers := Matchers{ - MatchLabelsMatcher: make(MatchLabelsMatcher), - } - - kindMatchers := make(kindMatchersMap) - - for _, comment := range r.headerComments { - if commentStartsWith(comment, "@kinds") { - err := appendKindMatchers(kindMatchers, comment) - if err != nil { - return Matchers{}, fmt.Errorf("get kind matchers: %w", err) - } - } - - if commentStartsWith(comment, "@matchlabels") { - m, err := getMatchLabelsMatcher(comment) - if err != nil { - return Matchers{}, fmt.Errorf("get match labels matcher: %w", err) - } - for k, v := range m { - matchers.MatchLabelsMatcher[k] = v - } - } - - if commentStartsWith(comment, "@matchExpression") { - m, err := getMatchExperssionsMatcher(comment) - if err != nil { - return Matchers{}, fmt.Errorf("get match expression matcher: %w", err) - } - matchers.MatchExpressionsMatcher = append(matchers.MatchExpressionsMatcher, m) - } - - if commentStartsWith(comment, "@namespaces") { - m, err := getStringListMatcher("@namespaces", comment) - if err != nil { - return Matchers{}, fmt.Errorf("get match namespaces matcher: %w", err) - } - matchers.NamespaceMatcher = append(matchers.NamespaceMatcher, m...) - } - - if commentStartsWith(comment, "@excludedNamespaces") { - m, err := getStringListMatcher("@excludedNamespaces", comment) - if err != nil { - return Matchers{}, fmt.Errorf("get match excludedNamespaces matcher: %w", err) - } - matchers.ExcludedNamespaceMatcher = append(matchers.ExcludedNamespaceMatcher, m...) - } - } - - if len(kindMatchers) > 0 { - matchers.KindMatchers = kindMatchers.convert() - } - - return matchers, nil -} - -func appendKindMatchers(kindMatchersMap kindMatchersMap, comment string) error { - kindMatcherText := strings.TrimSpace(strings.SplitAfter(comment, "@kinds")[1]) - kindMatcherGroups := strings.Split(kindMatcherText, " ") - - for _, kindMatcherGroup := range kindMatcherGroups { - kindMatcherSegments := strings.Split(kindMatcherGroup, "/") - if len(kindMatcherSegments) != 2 { - return fmt.Errorf("invalid @kinds: %s", kindMatcherGroup) - } - kindMatchersMap.addIfNotPreset(kindMatcherSegments[0], kindMatcherSegments[1]) - } - - return nil -} - -func getMatchLabelsMatcher(comment string) (MatchLabelsMatcher, error) { - matcher := make(map[string]string) - matcherText := strings.TrimSpace(strings.SplitAfter(comment, "@matchlabels")[1]) - for _, token := range strings.Fields(matcherText) { - split := strings.Split(token, "=") - if len(split) != 2 { - return nil, fmt.Errorf("invalid @matchlabels annotation token: %s", token) - } - - matcher[split[0]] = split[1] - } - - return matcher, nil -} - -func getMatchExperssionsMatcher(comment string) (MatchExpressionMatcher, error) { - argSpit := strings.TrimSpace(strings.SplitAfter(comment, "@matchExpression")[1]) - lineSplit := strings.Split(argSpit, " ") - if len(lineSplit) != 2 && len(lineSplit) != 3 { - return MatchExpressionMatcher{}, fmt.Errorf("too few parameters: have %d, need 2 or 3", len(lineSplit)) - } - matcher := MatchExpressionMatcher{ - Key: lineSplit[0], - Operator: lineSplit[1], - } - if len(lineSplit) == 3 { - matcher.Values = strings.Split(lineSplit[2], ",") - } - - return matcher, nil -} - -func getStringListMatcher(tag, comment string) ([]string, error) { - argSpit := strings.SplitAfter(comment, tag) - if len(argSpit) == 0 { - return nil, fmt.Errorf("no match for tag %q in comment %q", tag, comment) - } - lineSplit := strings.Split(strings.TrimSpace(argSpit[1]), " ") - if len(lineSplit) == 1 { - return nil, fmt.Errorf("no values provided for tag: %s", tag) - } - - return lineSplit, nil -} - -func commentStartsWith(comment string, keyword string) bool { - return strings.HasPrefix(strings.TrimSpace(comment), keyword) -} diff --git a/internal/rego/matchers_test.go b/internal/rego/matchers_test.go deleted file mode 100644 index 5b1646ac..00000000 --- a/internal/rego/matchers_test.go +++ /dev/null @@ -1,213 +0,0 @@ -package rego - -import ( - "reflect" - "testing" -) - -func TestMultilineKindMatchers(t *testing.T) { - comments := []string{ - "@kinds core/Pod apps/Deployment", - "@kinds apps/StatefulSet", - } - rego := Rego{ - headerComments: comments, - } - - expected := KindMatchers{ - {APIGroup: "", Kinds: []string{"Pod"}}, - {APIGroup: "apps", Kinds: []string{"Deployment", "StatefulSet"}}, - } - - matchers, err := rego.Matchers() - if err != nil { - t.Fatal(err) - } - actual := matchers.KindMatchers - - if !reflect.DeepEqual(expected, actual) { - t.Errorf("Unexpected KindMatchers. expected %v, actual %v.", expected, actual) - } -} - -func TestKindMatchers(t *testing.T) { - tests := []struct { - name string - comment string - want KindMatchers - }{ - { - "core_Pod", - "@kinds core/Pod", - KindMatchers{{APIGroup: "", Kinds: []string{"Pod"}}}, - }, - { - "core_Pod,core_Pod", - "@kinds core/Pod core/Pod", - KindMatchers{{APIGroup: "", Kinds: []string{"Pod"}}}, - }, - { - "apps_Deployment,apps_StatefulSet", - "@kinds apps/Deployment apps/StatefulSet", - KindMatchers{{APIGroup: "apps", Kinds: []string{"Deployment", "StatefulSet"}}}, - }, - { - "apps_StatefulSet,apps_Deployment", - "@kinds apps/StatefulSet apps/Deployment", - KindMatchers{{APIGroup: "apps", Kinds: []string{"Deployment", "StatefulSet"}}}, - }, - { - "apps_Deployment,apps_StatefulSet,core_Pod", - "@kinds apps/Deployment apps/StatefulSet core/Pod", - KindMatchers{ - {APIGroup: "", Kinds: []string{"Pod"}}, - {APIGroup: "apps", Kinds: []string{"Deployment", "StatefulSet"}}, - }, - }, - { - "apps_Deployment,core_Pod,apps_StatefulSet", - "@kinds apps/Deployment core/Pod apps/StatefulSet", - KindMatchers{ - {APIGroup: "", Kinds: []string{"Pod"}}, - {APIGroup: "apps", Kinds: []string{"Deployment", "StatefulSet"}}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - matchers, err := Rego{headerComments: []string{tt.comment}}.Matchers() - if err != nil { - t.Fatal(err) - } - actual := matchers.KindMatchers - if !reflect.DeepEqual(tt.want, actual) { - t.Errorf("Unexpected KindMatchers. expected %v, actual %v.", tt.want, actual) - } - }) - } -} - -func TestGetMatchLabelsMatcher(t *testing.T) { - comments := []string{ - "@matchlabels team=a app.kubernetes.io/name=test", - "@matchlabels example.com/env=production", - } - rego := Rego{ - headerComments: comments, - } - - expected := MatchLabelsMatcher{ - "team": "a", - "app.kubernetes.io/name": "test", - "example.com/env": "production", - } - - matchers, err := rego.Matchers() - if err != nil { - t.Fatal(err) - } - actual := matchers.MatchLabelsMatcher - - if !reflect.DeepEqual(expected, actual) { - t.Errorf("Unexpected MatchLabelMatcher. expected %v, actual %v.", expected, actual) - } -} - -func TestGetMatchExpressionsMatcher(t *testing.T) { - testCases := []struct { - name string - comments []string - expected []MatchExpressionMatcher - wantError bool - }{ - { - name: "Empty", - }, - { - name: "Single", - comments: []string{"@matchExpression foo In bar,baz"}, - expected: []MatchExpressionMatcher{ - {Key: "foo", Operator: "In", Values: []string{"bar", "baz"}}, - }, - }, - { - name: "DoubleWithUnrelated", - comments: []string{ - "@matchExpression foo In bar,baz", - "@matchExpression doggos Exists", - "unrelated comment", - }, - expected: []MatchExpressionMatcher{ - {Key: "foo", Operator: "In", Values: []string{"bar", "baz"}}, - {Key: "doggos", Operator: "Exists"}, - }, - }, - { - name: "TooFewParams", - comments: []string{ - "@matchExpression foo", - }, - wantError: true, - }, - { - name: "TooManyParams", - comments: []string{ - "@matchExpression foo In bar baz", - }, - wantError: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - rego := Rego{headerComments: tc.comments} - matchers, err := rego.Matchers() - if (err != nil && !tc.wantError) || (err == nil && tc.wantError) { - t.Errorf("Unexpected error state, have %v want %v", !tc.wantError, tc.wantError) - } - actual := matchers.MatchExpressionsMatcher - if !reflect.DeepEqual(actual, tc.expected) { - t.Errorf("Unexpected MatchExpressionsMatcher, have %v want %v", actual, tc.expected) - } - }) - } -} - -func TestGetStringListMatcher(t *testing.T) { - testCases := []struct { - desc string - tag string - comment string - expected []string - wantError bool - }{ - { - desc: "InvalidTag", - wantError: true, // will error without a match on the tag - }, - { - desc: "NoValuesSupplied", - tag: "@foo", - comment: "@foo ", - wantError: true, - }, - { - desc: "Valid", - tag: "@foo", - comment: "@foo bar baz", - expected: []string{"bar", "baz"}, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - actual, err := getStringListMatcher(tc.tag, tc.comment) - if (err != nil && !tc.wantError) || (err == nil && tc.wantError) { - t.Errorf("Unexpected error state, have %v want %v", !tc.wantError, tc.wantError) - } - if !reflect.DeepEqual(actual, tc.expected) { - t.Errorf("Unexpected values, have %v want %v", actual, tc.expected) - } - }) - } -} diff --git a/internal/rego/rego.go b/internal/rego/rego.go index e9c71966..bfe9f53c 100644 --- a/internal/rego/rego.go +++ b/internal/rego/rego.go @@ -42,6 +42,11 @@ const ( annoLabels = "labels" ) +const ( + coreAPIGroup = "core" + coreAPIShorthand = "" +) + type MetaData struct { Annotations map[string]string Labels map[string]string @@ -53,10 +58,8 @@ type Rego struct { path string raw string sanitizedRaw string - headerComments []string rules []string dependencies []string - parameters []Parameter enforcement string skipTemplate bool skipConstraint bool @@ -156,11 +159,6 @@ func (r Rego) Path() string { return r.path } -// Parameters returns the list of parsed parameters -func (r Rego) Parameters() []Parameter { - return r.parameters -} - func (r Rego) AnnotationKindMatchers() []AnnoKindMatcher { return r.annoKindMatchers } @@ -413,23 +411,7 @@ func (r Rego) Annotations() map[string]string { // Title returns the title found in the header comment of the rego file. func (r Rego) Title() string { - if r.annoTitle != "" { - return r.annoTitle - } - - var title string - for _, comment := range r.headerComments { - if !commentStartsWith(comment, "@title") { - continue - } - - title = strings.SplitAfter(comment, "@title")[1] - break - } - - title = strings.TrimSpace(title) - title = strings.Trim(title, "\n") - return title + return r.annoTitle } // Enforcement returns the enforcement action in the header comment. Defaults @@ -441,22 +423,6 @@ func (r Rego) Enforcement() string { return "deny" } -func getEnforcementTag(headerComments []string) string { - enforcement := "" - for _, comment := range headerComments { - if !commentStartsWith(comment, "@enforcement") { - continue - } - - enforcement = strings.SplitAfter(comment, "@enforcement")[1] - break - } - - enforcement = strings.TrimSpace(enforcement) - enforcement = strings.Trim(enforcement, "\n") - return enforcement -} - // PolicyID returns the identifier of the policy. The returned value will be a // blank string if an id was not specified in the policy body. func (r Rego) PolicyID() string { @@ -466,48 +432,7 @@ func (r Rego) PolicyID() string { // Description returns the entire description found in the header comment of // the Rego file. func (r Rego) Description() string { - if r.annoDescription != "" { - return r.annoDescription - } - - var description string - var handlingCodeBlock bool - var handlingParamDescription bool - - for _, comment := range r.headerComments { - if !handlingCodeBlock && !handlingParamDescription && commentStartsWith(comment, "@parameter") && strings.Contains(comment, "--") { - handlingParamDescription = true - } else if !handlingCodeBlock && handlingParamDescription && !commentStartsWith(comment, "--") { - handlingParamDescription = false - } - - if handlingParamDescription || commentStartsWith(comment, "@") { - continue - } - - // By default, we trim the comments found in the header to produce better looking documentation. - // However, when a comment in the Rego starts with a code block, we do not want to format - // any of the text within the code block. - if commentStartsWith(comment, "```") { - // Everytime we see a code block marker, we want to flip the status of whether or - // not we are currently handling a code block. - // - // i.e. The first time we see a codeblock marker we are handling a codeblock. - // The second time we see a codeblock marker, we are no longer handling that codeblock. - handlingCodeBlock = !handlingCodeBlock - } - - if handlingCodeBlock { - description += comment - } else { - description += strings.TrimSpace(comment) - } - - description += "\n" - } - - description = strings.Trim(description, "\n") - return description + return r.annoDescription } // HasMetadataAnnotations checks whether rego file has @@ -516,92 +441,6 @@ func (r Rego) HasMetadataAnnotations() bool { return r.annotations != nil } -// ConvertedLegacyAnnotations holds OPA Metadata Annotations, -// which were converted from legacy style annotations -type ConvertedLegacyAnnotations struct { - Title string `json:"title,omitempty"` - Description string `json:"description,omitempty"` - Custom map[string]any `json:"custom,omitempty"` -} - -// ConvertLegacyAnnotations converts legacy annotations to ConvertedLegacyAnnotations -func (r Rego) ConvertLegacyAnnotations() (*ConvertedLegacyAnnotations, error) { - custom := make(map[string]any) - if r.enforcement != "" { - custom[annoEnforcement] = r.enforcement - } - if r.skipTemplate { - custom[annoSkipTemplate] = r.skipTemplate - } - if r.skipConstraint { - custom[annoSkipConstraint] = r.skipConstraint - } - if len(r.parameters) > 0 { - custom[annoParameters] = r.GetOpenAPISchemaProperties() - } - - matchers, err := r.Matchers() - if err != nil { - return nil, fmt.Errorf("cant get legacy matchers: %w", err) - } - - matcherMap := make(map[string]any) - - if len(matchers.KindMatchers) > 0 { - matcherMap["kinds"] = matchers.KindMatchers.ToSpec() - } - - labelSelector := make(map[string]any) - if len(matchers.MatchLabelsMatcher) > 0 { - labelSelector["matchLabels"] = matchers.MatchLabelsMatcher - } - if len(matchers.MatchExpressionsMatcher) > 0 { - labelSelector["matchExpressions"] = matchers.MatchExpressionsMatcher - } - if len(labelSelector) > 0 { - matcherMap["labelSelector"] = labelSelector - } - - if len(matchers.NamespaceMatcher) > 0 { - matcherMap["namespaces"] = matchers.NamespaceMatcher - } - if len(matchers.ExcludedNamespaceMatcher) > 0 { - matcherMap["excludedNamespaces"] = matchers.ExcludedNamespaceMatcher - } - - if len(matcherMap) > 0 { - custom[annoMatchers] = matcherMap - } - - return &ConvertedLegacyAnnotations{ - Title: r.Title(), - Description: r.Description(), - Custom: custom, - }, nil -} - -func (r Rego) GetOpenAPISchemaProperties() map[string]apiextensionsv1.JSONSchemaProps { - properties := make(map[string]apiextensionsv1.JSONSchemaProps) - for _, p := range r.Parameters() { - if p.IsArray { - properties[p.Name] = apiextensionsv1.JSONSchemaProps{ - Type: "array", - Description: p.Description, - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{Type: p.Type}, - }, - } - } else { - properties[p.Name] = apiextensionsv1.JSONSchemaProps{ - Type: p.Type, - Description: p.Description, - } - } - } - - return properties -} - // Source returns the original source code inside // of the rego file without any comments. func (r Rego) Source() string { @@ -616,13 +455,6 @@ func (r Rego) FullSource() string { return strings.Trim(withoutHeader, "\n\t ") } -// LegacyConversionSource returns the original source code -// with comments except header, -// but doesn't trim any trailing whitespace -func (r Rego) LegacyConversionSource() string { - return removeHeaderComments(r.raw) -} - func removeHeaderComments(input string) string { var result string split := strings.Split(input, "\n") @@ -715,50 +547,24 @@ func parseDirectory(directory string, parseImports bool) ([]Rego, error) { } } - var headerComments []string - for _, c := range file.Parsed.Comments { - // If the line number of the comment comes before the line number - // that the package is declared on, we can safely assume that it is - // a header comment. - if c.Location.Row < file.Parsed.Package.Location.Row { - headerComments = append(headerComments, string(c.Text)) - } else { - break - } - } - bodyParams := getRuleParamNames(file.Parsed.Rules) - var paramsDiff []string - var headerParams []Parameter - if annotations == nil { - headerParams, err = getHeaderParamsLegacy(headerComments) - if err != nil { - return nil, fmt.Errorf("parse header parameters: %w", err) - } - paramsDiff = paramDiff(bodyParams, headerParams) - } else { + if annotations != nil { tempHeaderParams := getHeaderParams(annotations) - paramsDiff = paramDiff(bodyParams, tempHeaderParams) - } + paramsDiff := paramDiff(bodyParams, tempHeaderParams) + if len(paramsDiff) > 0 { + return nil, fmt.Errorf("missing definitions for parameters %v found in the policy `%s`", paramsDiff, file.Name) + } - if len(paramsDiff) > 0 { - return nil, fmt.Errorf("missing definitions for parameters %v found in the policy `%s`", paramsDiff, file.Name) } - rego := Rego{ - id: getPolicyID(file.Parsed.Rules), - path: file.Name, - dependencies: dependencies, - rules: rules, - parameters: headerParams, - headerComments: headerComments, - raw: string(file.Raw), - sanitizedRaw: sanitizeRawSource(file.Raw), - skipTemplate: hasSkipTemplateTag(headerComments), - skipConstraint: hasSkipConstraintTag(headerComments), - enforcement: getEnforcementTag(headerComments), - annotations: annotations, + id: getPolicyID(file.Parsed.Rules), + path: file.Name, + dependencies: dependencies, + rules: rules, + raw: string(file.Raw), + sanitizedRaw: sanitizeRawSource(file.Raw), + annotations: annotations, } if annotations != nil { @@ -766,7 +572,6 @@ func parseDirectory(directory string, parseImports bool) ([]Rego, error) { return nil, fmt.Errorf("parse OPA Metadata annotations: %w", err) } } - regos = append(regos, rego) } @@ -815,59 +620,6 @@ func getHeaderParams(annotations *ast.Annotations) []Parameter { return parameters } -func getHeaderParamsLegacy(comments []string) ([]Parameter, error) { - var parameters []Parameter - for i := 0; i < len(comments); i++ { - comment := comments[i] - - if !commentStartsWith(comment, "@parameter ") { - continue - } - - params := strings.SplitAfter(comment, "@parameter ")[1] - paramsDesc := strings.SplitN(params, " --", 2) - params = paramsDesc[0] - paramsSplit := strings.Split(params, " ") - if len(paramsSplit) == 0 { - return nil, fmt.Errorf("parameter name and type must be specified") - } - if len(paramsSplit) == 1 { - return nil, fmt.Errorf("type must be supplied with parameter name: %s", paramsSplit[0]) - } - - p := Parameter{Name: paramsSplit[0]} - if paramsSplit[1] == "array" { - if len(paramsSplit) == 2 { - return nil, fmt.Errorf("array type must be supplied with parameter name: %s", paramsSplit[0]) - } - p.IsArray = true - p.Type = paramsSplit[2] - } else { - p.Type = paramsSplit[1] - } - - if len(paramsDesc) > 1 { - p.Description = strings.TrimSpace(paramsDesc[1]) - - for i++; i != len(comments); i++ { - extraComment := strings.TrimSpace(comments[i]) - if !strings.HasPrefix(extraComment, "--") { - i-- - break - } - extraComment = strings.TrimSpace(extraComment[2:]) - p.Description += " " + extraComment - } - - p.Description = strings.TrimSpace(p.Description) - } - - parameters = append(parameters, p) - } - - return parameters, nil -} - func trimEachLine(raw string) string { var result string @@ -879,26 +631,6 @@ func trimEachLine(raw string) string { return result } -func hasSkipTemplateTag(comments []string) bool { - for _, comment := range comments { - if commentStartsWith(comment, "@skip-template") { - return true - } - } - - return false -} - -func hasSkipConstraintTag(comments []string) bool { - for _, comment := range comments { - if commentStartsWith(comment, "@skip-constraint") { - return true - } - } - - return false -} - func removeComments(raw string) string { var regoWithoutComments string lines := strings.Split(raw, "\n") diff --git a/internal/rego/rego_test.go b/internal/rego/rego_test.go index f4b5116b..b12e2973 100644 --- a/internal/rego/rego_test.go +++ b/internal/rego/rego_test.go @@ -34,32 +34,58 @@ func TestName(t *testing.T) { } func TestTitle(t *testing.T) { - comments := []string{ - "@title The title", - } + comments := ` +# METADATA +# title: The Title +# description: |- +# The description +# Extra comment +package foo +foo = "bar" { true } +` + rule, err := ast.ParseModuleWithOpts("", comments, ast.ParserOptions{ProcessAnnotation: true}) - rego := Rego{ - headerComments: comments, + if err != nil { + t.Errorf("Error parsing module: %s", err) } + rego := Rego{} + + err = rego.parseAnnotations(rule.Annotations[0]) + if err != nil { + t.Errorf("Error parsing annotations: %s", err) + } actual := rego.Title() - const expected = "The title" + const expected = "The Title" if actual != expected { t.Errorf("unexpected Title. expected %v, actual %v", expected, actual) } } func TestDescription(t *testing.T) { - comments := []string{ - "@title The title", - "The description", - "@kinds The kinds", - "Extra comment", + comments := ` +# METADATA +# title: The Title +# description: |- +# The description +# Extra comment +package foo +foo = "bar" { true } + ` + + rule, err := ast.ParseModuleWithOpts("", comments, ast.ParserOptions{ProcessAnnotation: true}) + + if err != nil { + t.Errorf("Error parsing module: %s", err) } - rego := Rego{ - headerComments: comments, + rego := Rego{} + + err = rego.parseAnnotations(rule.Annotations[0]) + + if err != nil { + t.Errorf("Error parsing annotations: %s", err) } actual := rego.Description() @@ -110,19 +136,6 @@ third` } func TestEnforcement(t *testing.T) { - comments := []string{ - "@title Test", - "description", - "@enforcement dryrun", - "@kinds apps/Deployment", - } - - actual := getEnforcementTag(comments) - const expected = "dryrun" - if actual != expected { - t.Errorf("unexpected Enforcement. expected %v, actual %v", expected, actual) - } - actualDefault := Rego{}.Enforcement() const expectedDefault = "deny" if actualDefault != expectedDefault { @@ -207,71 +220,3 @@ func TestGetRuleParamNamesFromInput(t *testing.T) { }) } } - -func TestGetHeaderParams(t *testing.T) { - comments := []string{ - "@title Title", - "Description", - "@parameter foo string -- with description", - "@parameter bar array string", - "@parameter baz array string -- with multiline", - "-- description", - "@kinds another/thing", - } - - expected := []Parameter{ - { - Name: "foo", - Type: "string", - Description: "with description", - }, - { - Name: "bar", - Type: "string", - IsArray: true, - }, - { - Name: "baz", - Type: "string", - IsArray: true, - Description: "with multiline description", - }, - } - - actual, err := getHeaderParamsLegacy(comments) - if err != nil { - t.Fatalf("get header params: %s", err) - } - - if !(reflect.DeepEqual(expected, actual)) { - t.Errorf("unexpected headerParams. expected %+v, actual %+v", expected, actual) - } -} - -func TestHasSkipTemplateTag(t *testing.T) { - comments := []string{ - "@title Title", - "Description", - "@kinds another/thing", - "@skip-template", - } - - skip := hasSkipTemplateTag(comments) - if !skip { - t.Error("SkipTemplate is false when the @skip-template comment tag is present") - } -} - -func TestHasSkipConstraintTag(t *testing.T) { - comments := []string{ - "@title Title", - "Description", - "@kinds another/thing", - "@skip-constraint", - } - - skip := hasSkipConstraintTag(comments) - if !skip { - t.Error("SkipConstraint is false when the @skip-constraint comment tag is present") - } -}