Skip to content

Commit

Permalink
UPSTREAM: <carry>: Extend NodeLogQuery feature
Browse files Browse the repository at this point in the history 
Extend the NodeLogQuery feature to support oc adm node-logs options:
- Default NodeLogQuery feature gate to true
- Add support for --since, --until, --case-sensitive, --output, options

UPSTREAM: <carry>: Extend NodeLogQuery feature

Fix handling of the "until" parameter when generating the journalctl
command. This was incorrectly being passed with the "since" value.
  • Loading branch information
@aravindhp @soltysh
aravindhp authored and soltysh committed Nov 3, 2023
1 parent 2e01b1c commit 940f406
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 41 deletions.
2 changes: 1 addition & 1 deletion pkg/features/kube_features.go
View file
Expand Up @@ -1128,7 +1128,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

NFTablesProxyMode: {Default: false, PreRelease: featuregate.Alpha},

NodeLogQuery: {Default: false, PreRelease: featuregate.Alpha},
NodeLogQuery: {Default: true, PreRelease: featuregate.Alpha},

NodeOutOfServiceVolumeDetach: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31

Expand Down
1 change: 1 addition & 0 deletions pkg/kubelet/apis/config/validation/validation_test.go
View file
Expand Up @@ -508,6 +508,7 @@ func TestValidateKubeletConfiguration(t *testing.T) {
}, {
name: "enableSystemLogQuery is enabled without NodeLogQuery feature gate",
configure: func(conf *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
conf.FeatureGates = map[string]bool{"NodeLogQuery": false}
conf.EnableSystemLogQuery = true
return conf
},
Expand Down
7 changes: 2 additions & 5 deletions pkg/kubelet/kubelet.go
View file
Expand Up @@ -1563,16 +1563,13 @@ func (kl *Kubelet) Run(updates <-chan kubetypes.PodUpdate) {
http.Error(w, errs.ToAggregate().Error(), http.StatusBadRequest)
return
} else if nlq != nil {
if req.URL.Path != "/" && req.URL.Path != "" {
http.Error(w, "path not allowed in query mode", http.StatusNotAcceptable)
return
}
if errs := nlq.validate(); len(errs) > 0 {
http.Error(w, errs.ToAggregate().Error(), http.StatusNotAcceptable)
return
}
// Validation ensures that the request does not query services and files at the same time
if len(nlq.Services) > 0 {
// OCP: Presence of journal in the path indicates it is a query for service(s)
if len(nlq.Services) > 0 || req.URL.Path == "journal" || req.URL.Path == "journal/" {
journal.ServeHTTP(w, req)
return
}
Expand Down
111 changes: 96 additions & 15 deletions pkg/kubelet/kubelet_server_journal.go
View file
Expand Up @@ -35,7 +35,7 @@ import (
"time"

securejoin "github.com/cyphar/filepath-securejoin"

"k8s.io/apimachinery/pkg/util/sets"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
)
Expand All @@ -54,6 +54,7 @@ var (
// character cannot be used to create invalid sequences. This is intended as a broad defense against malformed
// input that could cause an escape.
reServiceNameUnsafeCharacters = regexp.MustCompile(`[^a-zA-Z\-_.:0-9@]+`)
reRelativeDate = regexp.MustCompile(`^(\+|\-)?[\d]+(s|m|h|d)$`)
)

// journalServer returns text output from the OS specific service logger to view
Expand Down Expand Up @@ -114,6 +115,19 @@ type options struct {
// Pattern filters log entries by the provided regex pattern. On Linux nodes, this pattern will be read as a
// PCRE2 regex, on Windows nodes it will be read as a PowerShell regex. Support for this is implementation specific.
Pattern string
ocAdm
}

// ocAdm encapsulates the oc adm node-logs specific options
type ocAdm struct {
// Since is an ISO timestamp or relative date from which to show logs
Since string
// Until is an ISO timestamp or relative date until which to show logs
Until string
// Format is the alternate format (short, cat, json, short-unix) to display journal logs
Format string
// CaseSensitive controls the case sensitivity of pattern searches
CaseSensitive bool
}

// newNodeLogQuery parses query values and converts all known options into nodeLogQuery
Expand All @@ -122,7 +136,7 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
var nlq nodeLogQuery
var err error

queries, ok := query["query"]
queries, okQuery := query["query"]
if len(queries) > 0 {
for _, q := range queries {
// The presence of / or \ is a hint that the query is for a log file. If the query is for foo.log without a
Expand All @@ -134,11 +148,20 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
}
}
}
units, okUnit := query["unit"]
if len(units) > 0 {
for _, u := range units {
// We don't check for files as the heuristics do not apply to unit
if strings.TrimSpace(u) != "" { // Prevent queries with just spaces
nlq.Services = append(nlq.Services, u)
}
}
}

// Prevent specifying an empty or blank space query.
// Example: kubectl get --raw /api/v1/nodes/$node/proxy/logs?query=" "
if ok && (len(nlq.Files) == 0 && len(nlq.Services) == 0) {
allErrs = append(allErrs, field.Invalid(field.NewPath("query"), queries, "query cannot be empty"))
if (okQuery || okUnit) && (len(nlq.Files) == 0 && len(nlq.Services) == 0) {
allErrs = append(allErrs, field.Invalid(field.NewPath("unit"), queries, "unit cannot be empty"))
}

var sinceTime time.Time
Expand Down Expand Up @@ -176,6 +199,9 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {

var tailLines int
tailLinesValue := query.Get("tailLines")
if len(tailLinesValue) == 0 {
tailLinesValue = query.Get("tail")
}
if len(tailLinesValue) > 0 {
tailLines, err = strconv.Atoi(tailLinesValue)
if err != nil {
Expand All @@ -186,15 +212,28 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
}

pattern := query.Get("pattern")
if len(pattern) == 0 {
pattern = query.Get("grep")
}
if len(pattern) > 0 {
nlq.Pattern = pattern
caseSensitiveValue := query.Get("case-sensitive")
if len(caseSensitiveValue) > 0 {
caseSensitive, err := strconv.ParseBool(query.Get("case-sensitive"))
if err != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("case-sensitive"), query.Get("case-sensitive"),
err.Error()))
} else {
nlq.CaseSensitive = caseSensitive
}
}
}

if len(allErrs) > 0 {
return nil, allErrs
}
nlq.Since = query.Get("since")
nlq.Until = query.Get("until")
nlq.Format = query.Get("output")

if reflect.DeepEqual(nlq, nodeLogQuery{}) {
if len(allErrs) > 0 {
return nil, allErrs
}

Expand All @@ -219,14 +258,13 @@ func validateServices(services []string) field.ErrorList {
func (n *nodeLogQuery) validate() field.ErrorList {
allErrs := validateServices(n.Services)
switch {
case len(n.Files) == 0 && len(n.Services) == 0:
allErrs = append(allErrs, field.Required(field.NewPath("query"), "cannot be empty with options"))
// OCP: Allow len(n.Files) == 0 && len(n.Services) == 0 as we want to be able to return all journal / WinEvent logs
case len(n.Files) > 0 && len(n.Services) > 0:
allErrs = append(allErrs, field.Invalid(field.NewPath("query"), fmt.Sprintf("%v, %v", n.Files, n.Services),
"cannot specify a file and service"))
case len(n.Files) > 1:
allErrs = append(allErrs, field.Invalid(field.NewPath("query"), n.Files, "cannot specify more than one file"))
case len(n.Files) == 1 && n.options != (options{}):
case len(n.Files) == 1 && !reflect.DeepEqual(n.options, options{}):
allErrs = append(allErrs, field.Invalid(field.NewPath("query"), n.Files, "cannot specify file with options"))
case len(n.Files) == 1:
if fullLogFilename, err := securejoin.SecureJoin(nodeLogDir, n.Files[0]); err != nil {
Expand Down Expand Up @@ -258,6 +296,35 @@ func (n *nodeLogQuery) validate() field.ErrorList {
allErrs = append(allErrs, field.Invalid(field.NewPath("pattern"), n.Pattern, err.Error()))
}

// "oc adm node-logs" specific validation

if n.SinceTime != nil && (len(n.Since) > 0 || len(n.Until) > 0) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("sinceTime"),
"`since or until` and `sinceTime` cannot be specified"))
}

if n.UntilTime != nil && (len(n.Since) > 0 || len(n.Until) > 0) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("untilTime"),
"`since or until` and `untilTime` cannot be specified"))
}

if err := validateDate(n.Since); err != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("since"), n.Since, err.Error()))
}

if err := validateDate(n.Until); err != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("until"), n.Until, err.Error()))
}

allowedFormats := sets.New[string]("short-precise", "json", "short", "short-unix", "short-iso",
"short-iso-precise", "cat", "")
if len(n.Format) > 0 && runtime.GOOS == "windows" {
allErrs = append(allErrs, field.Invalid(field.NewPath("output"), n.Format,
"output is not supported on Windows"))
} else if !allowedFormats.Has(n.Format) {
allErrs = append(allErrs, field.NotSupported(field.NewPath("output"), n.Format, allowedFormats.UnsortedList()))
}

return allErrs
}

Expand All @@ -280,19 +347,20 @@ func (n *nodeLogQuery) copyForBoot(ctx context.Context, w io.Writer, previousBoo
return
}
nativeLoggers, fileLoggers := n.splitNativeVsFileLoggers(ctx)
if len(nativeLoggers) > 0 {
n.copyServiceLogs(ctx, w, nativeLoggers, previousBoot)
}

if len(fileLoggers) > 0 && n.options != (options{}) {
if len(fileLoggers) > 0 && !reflect.DeepEqual(n.options, options{}) {
fmt.Fprintf(w, "\noptions present and query resolved to log files for %v\ntry without specifying options\n",
fileLoggers)
return
}

if len(fileLoggers) > 0 {
copyFileLogs(ctx, w, fileLoggers)
return
}
// OCP: Return all logs in the case where nativeLoggers == ""
n.copyServiceLogs(ctx, w, nativeLoggers, previousBoot)

}

// splitNativeVsFileLoggers checks if each service logs to native OS logs or to a file and returns a list of services
Expand Down Expand Up @@ -442,3 +510,16 @@ func safeServiceName(s string) error {
}
return nil
}

func validateDate(date string) error {
if len(date) == 0 {
return nil
}
if reRelativeDate.MatchString(date) {
return nil
}
if _, err := time.Parse(dateLayout, date); err == nil {
return nil
}
return fmt.Errorf("date must be a relative time of the form '(+|-)[0-9]+(s|m|h|d)' or a date in 'YYYY-MM-DD HH:MM:SS' form")
}
21 changes: 18 additions & 3 deletions pkg/kubelet/kubelet_server_journal_linux.go
View file
Expand Up @@ -31,14 +31,20 @@ func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error)
args := []string{
"--utc",
"--no-pager",
"--output=short-precise",
}
if n.SinceTime != nil {

if len(n.Since) > 0 {
args = append(args, fmt.Sprintf("--since=%s", n.Since))
} else if n.SinceTime != nil {
args = append(args, fmt.Sprintf("--since=%s", n.SinceTime.Format(dateLayout)))
}
if n.UntilTime != nil {

if len(n.Until) > 0 {
args = append(args, fmt.Sprintf("--until=%s", n.Until))
} else if n.UntilTime != nil {
args = append(args, fmt.Sprintf("--until=%s", n.SinceTime.Format(dateLayout)))
}

if n.TailLines != nil {
args = append(args, "--pager-end", fmt.Sprintf("--lines=%d", *n.TailLines))
}
Expand All @@ -49,12 +55,21 @@ func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error)
}
if len(n.Pattern) > 0 {
args = append(args, "--grep="+n.Pattern)
args = append(args, fmt.Sprintf("--case-sensitive=%t", n.CaseSensitive))
}

if n.Boot != nil {
args = append(args, "--boot", fmt.Sprintf("%d", *n.Boot))
}

var output string
if len(n.Format) > 0 {
output = n.Format
} else {
output = "short-precise"
}
args = append(args, fmt.Sprintf("--output=%s", output))

return "journalctl", args, nil
}

Expand Down
51 changes: 36 additions & 15 deletions pkg/kubelet/kubelet_server_journal_test.go
View file
Expand Up @@ -73,18 +73,18 @@ func Test_newNodeLogQuery(t *testing.T) {
want *nodeLogQuery
wantErr bool
}{
{name: "empty", query: url.Values{}, want: nil},
{query: url.Values{"unknown": []string{"true"}}, want: nil},
{name: "empty", query: url.Values{}, want: &nodeLogQuery{}},
{query: url.Values{"unknown": []string{"true"}}, want: &nodeLogQuery{}},

{query: url.Values{"sinceTime": []string{""}}, want: nil},
{query: url.Values{"sinceTime": []string{""}}, want: &nodeLogQuery{}},
{query: url.Values{"sinceTime": []string{"2019-12-04 02:00:00"}}, wantErr: true},
{query: url.Values{"sinceTime": []string{"2019-12-04 02:00:00.000"}}, wantErr: true},
{query: url.Values{"sinceTime": []string{"2019-12-04 02"}}, wantErr: true},
{query: url.Values{"sinceTime": []string{"2019-12-04 02:00"}}, wantErr: true},
{query: url.Values{"sinceTime": []string{validTimeValue}},
want: &nodeLogQuery{options: options{SinceTime: &validT}}},

{query: url.Values{"untilTime": []string{""}}, want: nil},
{query: url.Values{"untilTime": []string{""}}, want: &nodeLogQuery{}},
{query: url.Values{"untilTime": []string{"2019-12-04 02:00:00"}}, wantErr: true},
{query: url.Values{"untilTime": []string{"2019-12-04 02:00:00.000"}}, wantErr: true},
{query: url.Values{"untilTime": []string{"2019-12-04 02"}}, wantErr: true},
Expand All @@ -98,7 +98,7 @@ func Test_newNodeLogQuery(t *testing.T) {

{query: url.Values{"pattern": []string{"foo"}}, want: &nodeLogQuery{options: options{Pattern: "foo"}}},

{query: url.Values{"boot": []string{""}}, want: nil},
{query: url.Values{"boot": []string{""}}, want: &nodeLogQuery{}},
{query: url.Values{"boot": []string{"0"}}, want: &nodeLogQuery{options: options{Boot: intPtr(0)}}},
{query: url.Values{"boot": []string{"-23"}}, want: &nodeLogQuery{options: options{Boot: intPtr(-23)}}},
{query: url.Values{"boot": []string{"foo"}}, wantErr: true},
Expand All @@ -111,6 +111,11 @@ func Test_newNodeLogQuery(t *testing.T) {
{query: url.Values{"query": []string{"foo", "/bar"}}, want: &nodeLogQuery{Services: []string{"foo"},
Files: []string{"/bar"}}},
{query: url.Values{"query": []string{"/foo", `\bar`}}, want: &nodeLogQuery{Files: []string{"/foo", `\bar`}}},
{query: url.Values{"unit": []string{""}}, wantErr: true},
{query: url.Values{"unit": []string{" ", " "}}, wantErr: true},
{query: url.Values{"unit": []string{"foo"}}, want: &nodeLogQuery{Services: []string{"foo"}}},
{query: url.Values{"unit": []string{"foo", "bar"}}, want: &nodeLogQuery{Services: []string{"foo", "bar"}}},
{query: url.Values{"unit": []string{"foo", "/bar"}}, want: &nodeLogQuery{Services: []string{"foo", "/bar"}}},
}
for _, tt := range tests {
t.Run(tt.query.Encode(), func(t *testing.T) {
Expand Down Expand Up @@ -173,10 +178,12 @@ func Test_nodeLogQuery_validate(t *testing.T) {
pattern = "foo"
invalid = "foo\\"
)
since, err := time.Parse(time.RFC3339, "2023-01-04T02:00:00Z")
sinceTime, err := time.Parse(time.RFC3339, "2023-01-04T02:00:00Z")
assert.NoError(t, err)
until, err := time.Parse(time.RFC3339, "2023-02-04T02:00:00Z")
untilTime, err := time.Parse(time.RFC3339, "2023-02-04T02:00:00Z")
assert.NoError(t, err)
since := "2019-12-04 02:00:00"
until := "2019-12-04 03:00:00"

tests := []struct {
name string
Expand All @@ -185,23 +192,37 @@ func Test_nodeLogQuery_validate(t *testing.T) {
options options
wantErr bool
}{
{name: "empty", wantErr: true},
{name: "empty with options", options: options{SinceTime: &since}, wantErr: true},
{name: "empty"},
{name: "empty with options", options: options{SinceTime: &sinceTime}},
{name: "one service", Services: []string{service1}},
{name: "two services", Services: []string{service1, service2}},
{name: "one service one file", Services: []string{service1}, Files: []string{file1}, wantErr: true},
{name: "two files", Files: []string{file1, file2}, wantErr: true},
{name: "one file options", Files: []string{file1}, options: options{Pattern: pattern}, wantErr: true},
{name: "invalid pattern", Services: []string{service1}, options: options{Pattern: invalid}, wantErr: true},
{name: "since", Services: []string{service1}, options: options{SinceTime: &since}},
{name: "until", Services: []string{service1}, options: options{UntilTime: &until}},
{name: "since until", Services: []string{service1}, options: options{SinceTime: &until, UntilTime: &since},
wantErr: true},
// boot is not supported on Windows.
{name: "boot", Services: []string{service1}, options: options{Boot: intPtr(-1)}, wantErr: runtime.GOOS == "windows"},
{name: "sinceTime", Services: []string{service1}, options: options{SinceTime: &sinceTime}},
{name: "untilTime", Services: []string{service1}, options: options{UntilTime: &untilTime}},
{name: "sinceTime untilTime", Services: []string{service1}, options: options{SinceTime: &untilTime,
UntilTime: &sinceTime}, wantErr: true},
{name: "boot", Services: []string{service1}, options: options{Boot: intPtr(-1)}},
{name: "boot out of range", Services: []string{service1}, options: options{Boot: intPtr(1)}, wantErr: true},
{name: "tailLines", Services: []string{service1}, options: options{TailLines: intPtr(100)}},
{name: "tailLines out of range", Services: []string{service1}, options: options{TailLines: intPtr(100000)}},
{name: "since", Services: []string{service1}, options: options{ocAdm: ocAdm{Since: since}}},
{name: "since RFC3339", Services: []string{service1}, options: options{ocAdm: ocAdm{Since: sinceTime.String()}}, wantErr: true},
{name: "until", Services: []string{service1}, options: options{ocAdm: ocAdm{Until: until}}},
{name: "until RFC3339", Services: []string{service1}, options: options{ocAdm: ocAdm{Until: untilTime.String()}}, wantErr: true},
{name: "since sinceTime", Services: []string{service1}, options: options{SinceTime: &sinceTime,
ocAdm: ocAdm{Since: since}}, wantErr: true},
{name: "until sinceTime", Services: []string{service1}, options: options{SinceTime: &sinceTime,
ocAdm: ocAdm{Until: until}}, wantErr: true},
{name: "since untilTime", Services: []string{service1}, options: options{UntilTime: &untilTime,
ocAdm: ocAdm{Since: since}}, wantErr: true},
{name: "until untilTime", Services: []string{service1}, options: options{UntilTime: &untilTime,
ocAdm: ocAdm{Until: until}}, wantErr: true},
{name: "format", Services: []string{service1}, options: options{ocAdm: ocAdm{Format: "cat"}}},
{name: "format invalid", Services: []string{service1}, options: options{ocAdm: ocAdm{Format: "foo"}},
wantErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 940f406

Please sign in to comment.