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

Stricter linter and more godoc #24

Merged
merged 2 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,20 +97,19 @@ func init() {
AddRakkessFlags(rootCmd)

rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
if err := SetUpLogs(rakkessOptions.Streams.ErrOut, v); err != nil {
return err
}
return nil
return SetUpLogs(rakkessOptions.Streams.ErrOut, v)
}
}

// AddRakkessFlags sets up common flags for subcommands.
func AddRakkessFlags(cmd *cobra.Command) {
cmd.Flags().StringSliceVar(&rakkessOptions.Verbs, "verbs", []string{"list", "create", "update", "delete"}, fmt.Sprintf("show access for verbs out of %s", constants.ValidVerbs))
cmd.Flags().StringVarP(&rakkessOptions.OutputFormat, "output", "o", "icon-table", fmt.Sprintf("output format out of %s", constants.ValidOutputFormats))

rakkessOptions.ConfigFlags.AddFlags(cmd.Flags())
}

// SetUpLogs configures the loglevel and output writer for logs.
func SetUpLogs(out io.Writer, level string) error {
logrus.SetOutput(out)
lvl, err := logrus.ParseLevel(level)
Expand Down
4 changes: 4 additions & 0 deletions hack/run_lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,14 @@ fi
-E goimports \
-E golint \
-E gosec \
-E gosimple \
-E interfacer \
-E maligned \
-E misspell \
-E unconvert \
-E unparam \
-E stylecheck \
-E staticcheck \
-E structcheck \
-E prealloc \
--skip-dirs hack
1 change: 1 addition & 0 deletions pkg/rakkess/client/fetch_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (g GroupResource) fullName() string {
return fmt.Sprintf("%s.%s", g.APIResource.Name, g.APIGroup)
}

// FetchAvailableGroupResources fetches a list of known APIResources on the server.
func FetchAvailableGroupResources(opts *options.RakkessOptions) ([]GroupResource, error) {
client, err := opts.DiscoveryClient()
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions pkg/rakkess/client/resource_access.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ import (

var (
// for testing
getRbacClient = GetRbacClient
getRbacClient = getRbacClientImpl
)

const (
clusterRoleName = "ClusterRole"
roleName = "Role"
)

// GetSubjectAccess determines subjects with access to the given resource.
func GetSubjectAccess(opts *options.RakkessOptions, resource string) (*result.SubjectAccess, error) {
rbacClient, err := getRbacClient(opts)
if err != nil {
Expand Down Expand Up @@ -142,7 +143,7 @@ func fetchMatchingRoles(rbacClient clientv1.RolesGetter, sa *result.SubjectAcces
return nil
}

func GetRbacClient(o *options.RakkessOptions) (clientv1.RbacV1Interface, error) {
func getRbacClientImpl(o *options.RakkessOptions) (clientv1.RbacV1Interface, error) {
restConfig, err := o.ConfigFlags.ToRESTConfig()
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/rakkess/client/resource_access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestGetSubjectAccess(t *testing.T) {
getRbacClient = func(*options.RakkessOptions) (clientv1.RbacV1Interface, error) {
return fakeRbacClient, nil
}
defer func() { getRbacClient = GetRbacClient }()
defer func() { getRbacClient = getRbacClientImpl }()

opts := &options.RakkessOptions{
ConfigFlags: &genericclioptions.ConfigFlags{
Expand Down
11 changes: 9 additions & 2 deletions pkg/rakkess/client/result/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,20 @@ import (
"strings"
)

// ResourceAccessItem holds the access result for a resource.
type ResourceAccessItem struct {
Name string
// Name is the resource name.
Name string
// Access maps from verb to access code.
Access map[string]int
Err []error
// Err holds access error if a SelfSubjectAccessReview failed.
Err []error
}

// ResourceAccess holds the access result for all resources.
type ResourceAccess []ResourceAccessItem

// NewResourceAccess creates a fresh ResourceAccess and sorts the results by resource name.
func NewResourceAccess(items []ResourceAccessItem) ResourceAccess {
ra := ResourceAccess(items)
sort.Stable(ra)
Expand All @@ -49,6 +55,7 @@ func (ra ResourceAccess) Less(i, j int) bool {
return true
}

// Print implements MatrixPrinter.Print. It prints a tab-separated table with a header.
func (ra ResourceAccess) Print(w io.Writer, converter CodeConverter, requestedVerbs []string) {
// table header
fmt.Fprint(w, "NAME")
Expand Down
1 change: 1 addition & 0 deletions pkg/rakkess/client/result/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package result

import "io"

// This encodes the access of the given subject to the resource+verb combination.
const (
AccessAllowed = iota
AccessDenied
Expand Down
22 changes: 20 additions & 2 deletions pkg/rakkess/client/result/subject.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ type SubjectRef struct {
Name, Kind, Namespace string
}

// SubjectAccess holds the access information of all subjects for the given resource.
type SubjectAccess struct {
Resource string
roles map[RoleRef]sets.String
// Resource is the kubernetes resource that this instance applies to.
Resource string
// roles holds all rule data concerning this resource and is extracted from Roles and ClusterRoles.
roles map[RoleRef]sets.String
// roles holds all subject access data for this resource and is extracted from RoleBindings and ClusterRoleBindings.
subjectAccess map[SubjectRef]sets.String
}

// NewSubjectAccess creates a new SubjectAccess with initialized fields.
func NewSubjectAccess(resource string) *SubjectAccess {
return &SubjectAccess{
Resource: resource,
Expand All @@ -52,10 +57,19 @@ func NewSubjectAccess(resource string) *SubjectAccess {
}
}

// Get provides access to the actual result (for testing).
func (sa *SubjectAccess) Get() map[SubjectRef]sets.String {
return sa.subjectAccess
}

// Empty checks if any subjects with access were found.
func (sa *SubjectAccess) Empty() bool {
return len(sa.subjectAccess) == 0
}

// ResolveRoleRef takes a RoleRef and a list of subjects and stores the access
// rights of the given role for each subject. The RoleRef and subjects usually
// come from a (Cluster)RoleBinding.
func (sa *SubjectAccess) ResolveRoleRef(r RoleRef, subjects []v1.Subject) {
verbsForRole, ok := sa.roles[r]
if !ok {
Expand All @@ -75,6 +89,9 @@ func (sa *SubjectAccess) ResolveRoleRef(r RoleRef, subjects []v1.Subject) {
}
}

// MatchRules takes a RoleRef and a PolicyRule and adds the rule verbs to the
// allowed verbs for the RoleRef, if the sa.resource matches the rule.
// The RoleRef and rule usually come from a (Cluster)Role.
func (sa *SubjectAccess) MatchRules(r RoleRef, rule v1.PolicyRule) {
for _, resource := range rule.Resources {
if resource == v1.ResourceAll || resource == sa.Resource {
Expand All @@ -97,6 +114,7 @@ func expandVerbs(verbs []string) []string {
return verbs
}

// Print implements MatrixPrinter.Print. It prints a tab-separated table with a header.
func (sa *SubjectAccess) Print(w io.Writer, converter CodeConverter, requestedVerbs []string) {
// table header
fmt.Fprint(w, "NAME\tKIND\tSA-NAMESPACE")
Expand Down
2 changes: 1 addition & 1 deletion pkg/rakkess/client/result/subject_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func TestSortableSubjects(t *testing.T) {
}

func makeSubjects(in []string) []v1.Subject {
var subjects []v1.Subject
subjects := make([]v1.Subject, 0, len(in))
for _, s := range in {
subjects = append(subjects, v1.Subject{
Name: s,
Expand Down
7 changes: 5 additions & 2 deletions pkg/rakkess/client/user_access.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ import (
authv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
)

// todo(corneliusweig) error is always nil
// CheckResourceAccess determines the access rights for the given GroupResources and verbs.
// Since it needs to do a lot of requests, the SelfSubjectAccessReviewInterface needs to
// be configured for high queries per second.
func CheckResourceAccess(ctx context.Context, sar authv1.SelfSubjectAccessReviewInterface, grs []GroupResource, verbs []string, namespace *string) (result.MatrixPrinter, error) {
var results []result.ResourceAccessItem
results := make([]result.ResourceAccessItem, 0, len(grs))
group := sync.WaitGroup{}
semaphore := make(chan struct{}, 20)
resultsChan := make(chan result.ResourceAccessItem)
Expand Down Expand Up @@ -120,6 +122,7 @@ func CheckResourceAccess(ctx context.Context, sar authv1.SelfSubjectAccessReview
results = append(results, gr)
}

// todo(corneliusweig) error is always nil
return result.NewResourceAccess(results), nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/rakkess/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ package constants
import "github.com/sirupsen/logrus"

const (
// DefaultLogLevel is set to warn by default.
DefaultLogLevel = logrus.WarnLevel
)

var (
// ValidVerbs is the list of allowed actions on kubernetes resources.
ValidVerbs = []string{
"create",
"delete",
Expand All @@ -33,6 +35,8 @@ var (
"update",
"watch",
}

// ValidOutputFormats is the list of valid formats for the result table.
ValidOutputFormats = []string{
"icon-table",
"ascii-table",
Expand Down
4 changes: 4 additions & 0 deletions pkg/rakkess/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ import (
v1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
)

// RakkessOptions holds all user configuration options.
type RakkessOptions struct {
ConfigFlags *genericclioptions.ConfigFlags
Verbs []string
OutputFormat string
Streams *genericclioptions.IOStreams
}

// NewRakkessOptions creates RakkessOptions with defaults.
func NewRakkessOptions() *RakkessOptions {
return &RakkessOptions{
ConfigFlags: genericclioptions.NewConfigFlags(true),
Expand All @@ -42,6 +44,7 @@ func NewRakkessOptions() *RakkessOptions {
}
}

// GetAuthClient creates a client for SelfSubjectAccessReviews with high queries per second.
func (o *RakkessOptions) GetAuthClient() (v1.SelfSubjectAccessReviewInterface, error) {
restConfig, err := o.ConfigFlags.ToRESTConfig()
if err != nil {
Expand All @@ -55,6 +58,7 @@ func (o *RakkessOptions) GetAuthClient() (v1.SelfSubjectAccessReviewInterface, e
return authClient.SelfSubjectAccessReviews(), nil
}

// DiscoveryClient creates a kubernetes discovery client.
func (o *RakkessOptions) DiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
return o.ConfigFlags.ToDiscoveryClient()
}
5 changes: 3 additions & 2 deletions pkg/rakkess/printer/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,19 @@ const (
)

var (
IsTerminal = isTerminal
isTerminal = isTerminalImpl
terminit sync.Once
)

// PrintResults configures the table style and delegates printing to result.MatrixPrinter.
func PrintResults(out io.Writer, requestedVerbs []string, outputFormat string, results result.MatrixPrinter) {
w := NewWriter(out, 4, 8, 2, ' ', CollapseEscape|StripEscape)
defer w.Flush()

terminit.Do(func() { initTerminal(out) })

codeConverter := humanreadableAccessCode
if IsTerminal(out) {
if isTerminal(out) {
codeConverter = colorHumanreadableAccessCode
}
if outputFormat == "ascii-table" {
Expand Down
5 changes: 2 additions & 3 deletions pkg/rakkess/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,11 @@ func TestPrintResults(t *testing.T) {
}

for _, test := range tests[0:4] {
isTerminal := IsTerminal
IsTerminal = func(w io.Writer) bool {
isTerminal = func(w io.Writer) bool {
return true
}
defer func() {
IsTerminal = isTerminal
isTerminal = isTerminalImpl
}()

t.Run(test.name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/rakkess/printer/terminal_notwindows.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
func initTerminal(_ io.Writer) {
}

func isTerminal(w io.Writer) bool {
func isTerminalImpl(w io.Writer) bool {
if f, ok := w.(*os.File); ok {
return terminal.IsTerminal(int(f.Fd()))
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/rakkess/rakkess.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
)

// Resource determines the access right of the current (or impersonated) user
// and prints the result as a matrix with verbs in the horizontal and resource names
// in the vertical direction.
func Resource(ctx context.Context, opts *options.RakkessOptions) error {
if err := validation.Options(opts); err != nil {
return err
Expand Down Expand Up @@ -60,6 +63,9 @@ func Resource(ctx context.Context, opts *options.RakkessOptions) error {
return nil
}

// Subject determines the subjects with access right to the given resource and
// prints the result as a matrix with verbs in the horizontal and subject names
// in the vertical direction.
func Subject(opts *options.RakkessOptions, resource string) error {
if err := validation.Options(opts); err != nil {
return err
Expand All @@ -79,7 +85,7 @@ func Subject(opts *options.RakkessOptions, resource string) error {
return errors.Wrap(err, "get subject access")
}

if len(subjectAccess.Get()) == 0 {
if subjectAccess.Empty() {
logrus.Warnf("No subjects with access found. This most likely means that you have insufficient rights to review authorization.")
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/rakkess/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
)

// Options validates RakkessOptions. Fields validated:
// - OutputFormat
// - Verbs
func Options(opts *options.RakkessOptions) error {
if err := verbs(opts.Verbs); err != nil {
return err
}
if err := outputFormat(opts.OutputFormat); err != nil {
return err
}
return nil
return outputFormat(opts.OutputFormat)
}

func outputFormat(format string) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/rakkess/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
var version, gitCommit, buildDate string
var platform = fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH)

// BuildInfo stores static build information about the binary.
type BuildInfo struct {
BuildDate string
Compiler string
Expand All @@ -49,6 +50,7 @@ func GetBuildInfo() *BuildInfo {
}
}

// ParseVersion parses a version string ignoring a leading `v`. For example: v1.2.3
func ParseVersion(version string) (semver.Version, error) {
version = strings.TrimLeft(strings.TrimSpace(version), "v")
return semver.Parse(version)
Expand Down