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

(spyglass) allow configuration of sandbox permissions per lense - try 2 #392

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
4 changes: 4 additions & 0 deletions cmd/checkconfig/testdata/combined.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ deck:
- (FAIL|Failure \[)\b
- panic\b
- ^E\d{4} \d\d:\d\d:\d\d\.\d\d\d]
iframe_sandbox_permissions:
- allow-scripts
- allow-popups
- allow-popups-to-escape-sandbox
required_files:
- build-log.txt
- lens:
Expand Down
5 changes: 5 additions & 0 deletions cmd/deck/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1671,6 +1671,11 @@ func defaultLensRemoteConfig(lfc *config.LensFileConfig) error {
lfc.RemoteConfig.HideTitle = &hideTitle
}

if lfc.RemoteConfig.IframeSandboxPermissions == nil {
permissions := lens.Config().IframeSandboxPermissions
lfc.RemoteConfig.IframeSandboxPermissions = &permissions
}

return nil
}

Expand Down
8 changes: 8 additions & 0 deletions cmd/deck/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,9 @@ func verifyCfgHasRemoteForLens(lensName string) func(*config.Config, error) erro
if lens.RemoteConfig.HideTitle == nil {
return errors.New("expected HideTitle to be set")
}
if lens.RemoteConfig.IframeSandboxPermissions == nil {
return errors.New("expected IFrameSandboxPermissions to be set")
}
}

if !found {
Expand All @@ -990,6 +993,11 @@ func verifyCfgHasRemoteForLens(lensName string) func(*config.Config, error) erro

}

// TODO norrs: catch https://github.com/kubernetes-sigs/prow/issues/369
func TestSpyglassConfigNorrsCreatesDefaultIframeSandboxPermissions(t *testing.T) {
t.Fatalf("unfinished test")
}

func TestSpyglassConfigDefaulting(t *testing.T) {
t.Parallel()

Expand Down
2 changes: 1 addition & 1 deletion cmd/deck/template/spyglass.html
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
<div class="mdl-card__title lens-title"><h3 class="mdl-card__title-text">{{$config.Title}}</h3></div>
<div id="{{$config.Name}}-view-container" class="lens-view-content mdl-card__supporting-text">
<img src="/static/kubernetes-wheel.svg?v={{deckVersion}}" alt="loading spinner" class="loading-spinner is-active lens-card-loading" id="{{$config.Name}}-loading">
<iframe class="lens-container" style="visibility: hidden;" id="iframe-{{$index}}" sandbox="allow-scripts allow-top-navigation allow-popups allow-same-origin" data-lens-index="{{$index}}" data-lens-name="{{$config.Name}}"{{if $config.HideTitle}} data-hide-title="true"{{end}}></iframe>
<iframe class="lens-container" style="visibility: hidden;" id="iframe-{{$index}}" sandbox="{{$config.IframeSandboxPermissions}}" data-lens-index="{{$index}}" data-lens-name="{{$config.Name}}"{{if $config.HideTitle}} data-hide-title="true"{{end}}></iframe>
</div>
</div>
{{end}}
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,8 @@ type LensRemoteConfig struct {
Priority *uint `json:"priority"`
// HideTitle defines if we will keep showing the title after lens loads.
HideTitle *bool `json:"hide_title"`
// Iframe sandbox permissions to be configured for the lens's iframe.
IframeSandboxPermissions *[]string `json:"iframe_sandbox_permissions"`
}

// Spyglass holds config for Spyglass.
Expand Down
40 changes: 27 additions & 13 deletions pkg/spyglass/lenses/buildlog/lens.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ const (
var defaultHighlightLineLengthMax = 10000 // Default maximum length of a line worth highlighting

type config struct {
HighlightRegexes []string `json:"highlight_regexes"`
HideRawLog bool `json:"hide_raw_log,omitempty"`
Highlighter *highlightConfig `json:"highlighter,omitempty"`
HighlightLengthMax *int `json:"highlight_line_length_max,omitempty"`
HighlightRegexes []string `json:"highlight_regexes"`
HideRawLog bool `json:"hide_raw_log,omitempty"`
Highlighter *highlightConfig `json:"highlighter,omitempty"`
HighlightLengthMax *int `json:"highlight_line_length_max,omitempty"`
IframeSandboxPermissions []string `json:"iframe_sandbox_permissions,omitempty"`
}

type highlightConfig struct {
Expand All @@ -68,10 +69,11 @@ type highlightConfig struct {
}

type parsedConfig struct {
highlightRegex *regexp.Regexp
showRawLog bool
highlighter *highlightConfig
highlightLengthMax int
highlightRegex *regexp.Regexp
showRawLog bool
highlighter *highlightConfig
highlightLengthMax int
IframeSandboxPermissions string
}

var _ api.Lens = Lens{}
Expand All @@ -82,9 +84,10 @@ type Lens struct{}
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Name: name,
Title: title,
Priority: priority,
Name: name,
Title: title,
Priority: priority,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand All @@ -97,6 +100,9 @@ func (lens Lens) Header(artifacts []api.Artifact, resourceDir string, config jso
// It is only used if highlight_regexes is not specified in the lens config.
var defaultErrRE = regexp.MustCompile(`timed out|ERROR:|(FAIL|Failure \[)\b|panic\b|^E\d{4} \d\d:\d\d:\d\d\.\d\d\d]`)

// see defaultSandboxPermissions - this is string version for the html.
var defaultIframeSandboxPermissionsString = strings.Join(lenses.DefaultSandboxPermissions, " ")

func init() {
lenses.RegisterLens(Lens{})
}
Expand Down Expand Up @@ -170,8 +176,9 @@ type buildLogsView struct {

func getConfig(rawConfig json.RawMessage) parsedConfig {
conf := parsedConfig{
highlightRegex: defaultErrRE,
showRawLog: true,
highlightRegex: defaultErrRE,
showRawLog: true,
IframeSandboxPermissions: defaultIframeSandboxPermissionsString,
}

// No config at all is fine.
Expand All @@ -189,6 +196,13 @@ func getConfig(rawConfig json.RawMessage) parsedConfig {
conf.highlighter = nil
}
conf.showRawLog = !c.HideRawLog

if c.IframeSandboxPermissions == nil {
conf.IframeSandboxPermissions = defaultIframeSandboxPermissionsString
} else {
conf.IframeSandboxPermissions = strings.Join(c.IframeSandboxPermissions, " ")
}

if len(c.HighlightRegexes) == 0 {
return conf
}
Expand Down
19 changes: 18 additions & 1 deletion pkg/spyglass/lenses/buildlog/lens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ import (

func TestGetConfig(t *testing.T) {
def := parsedConfig{
showRawLog: true,
showRawLog: true,
IframeSandboxPermissions: defaultIframeSandboxPermissionsString,
}
cases := []struct {
name string
Expand All @@ -61,6 +62,22 @@ func TestGetConfig(t *testing.T) {
}
return d
}(),
}, {
name: "configure iframe sandbox permissions",
raw: `{"iframe_sandbox_permissions": ["allow-scripts", "allow-downloads"]}`,
want: func() parsedConfig {
d := def
d.IframeSandboxPermissions = "allow-scripts allow-downloads"
return d
}(),
}, {
name: "empty iframe sandbox permissions does not return default permissions",
raw: `{"iframe_sandbox_permissions": []}`,
want: func() parsedConfig {
d := def
d.IframeSandboxPermissions = ""
return d
}(),
},
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/spyglass/lenses/coverage/coverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ type Lens struct{}
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Name: name,
Title: title,
Priority: priority,
Name: name,
Title: title,
Priority: priority,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/spyglass/lenses/html/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,11 @@ type document struct {
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Name: "html",
Title: "HTML",
Priority: 3,
HideTitle: true,
Name: "html",
Title: "HTML",
Priority: 3,
HideTitle: true,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/spyglass/lenses/junit/lens.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ type JVD struct {
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Name: name,
Title: title,
Priority: priority,
Name: name,
Title: title,
Priority: priority,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
10 changes: 10 additions & 0 deletions pkg/spyglass/lenses/lenses.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ var (
ErrContextUnsupported = errors.New("artifact does not support context operations")
)

// DefaultSandboxPermissions is the default value for iframe_sandbox_permissions lense config if it is not specified
var DefaultSandboxPermissions = []string{
"allow-scripts",
"allow-top-navigation",
"allow-popups",
"allow-same-origin",
}

type LensConfig struct {
// Name is the name of the lens. It must match the package name.
Name string
Expand All @@ -60,6 +68,8 @@ type LensConfig struct {
Priority uint
// HideTitle will hide the lens title after loading if set to true.
HideTitle bool
// Iframe sandbox permissions to be configured for the lens's iframe.
IframeSandboxPermissions []string
}

// Lens defines the interface that lenses are required to implement in order to be used by Spyglass.
Expand Down
7 changes: 4 additions & 3 deletions pkg/spyglass/lenses/links/links.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ type Lens struct{}
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Name: name,
Title: title,
Priority: priority,
Name: name,
Title: title,
Priority: priority,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/spyglass/lenses/metadata/lens.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ func init() {
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Title: title,
Name: name,
Priority: priority,
HideTitle: true,
Title: title,
Name: name,
Priority: priority,
HideTitle: true,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/spyglass/lenses/podinfo/podinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,10 @@ type Lens struct{}
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Name: name,
Title: title,
Priority: priority,
Name: name,
Title: title,
Priority: priority,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/spyglass/lenses/restcoverage/restcoverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ func init() {
// Config returns the lens's configuration.
func (lens Lens) Config() lenses.LensConfig {
return lenses.LensConfig{
Title: "REST API coverage report",
Name: "restcoverage",
Priority: 0,
Title: "REST API coverage report",
Name: "restcoverage",
Priority: 0,
IframeSandboxPermissions: lenses.DefaultSandboxPermissions,
}
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/spyglass/spyglass.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ func getLensConfig(lensFileConfig config.LensFileConfig) (LensConfig, error) {
if lensFileConfig.RemoteConfig.HideTitle != nil {
lc.HideTitle = *lensFileConfig.RemoteConfig.HideTitle
}
if lensFileConfig.RemoteConfig.IframeSandboxPermissions != nil {
lc.IframeSandboxPermissions = *lensFileConfig.RemoteConfig.IframeSandboxPermissions
} else {
// we do not want to break existing remote config which doesn't have iframeSandboxPermissions set.
lc.IframeSandboxPermissions = lenses.DefaultSandboxPermissions
}
return lensConfigWrapper{lc}, nil
}
return nil, fmt.Errorf("could not find lens")
Expand Down