diff --git a/service/internal/api/api.go b/service/internal/api/api.go index 8eecbe9..c32292f 100644 --- a/service/internal/api/api.go +++ b/service/internal/api/api.go @@ -464,6 +464,53 @@ func (api *oliveTinAPI) GetLogs(ctx ctx.Context, req *connect.Request[apiv1.GetL return connect.NewResponse(ret), nil } +// isValidLogEntry checks if a log entry has all required fields populated. +func isValidLogEntry(e *executor.InternalLogEntry) bool { + return e != nil && e.Binding != nil && e.Binding.Action != nil +} + +// isLogEntryAllowed checks if a log entry is allowed to be viewed by the user. +func (api *oliveTinAPI) isLogEntryAllowed(e *executor.InternalLogEntry, user *acl.AuthenticatedUser) bool { + return acl.IsAllowedLogs(api.cfg, user, e.Binding.Action) +} + +// buildEmptyPageResponse creates a response for an empty page. +func buildEmptyPageResponse(page pageInfo) *apiv1.GetActionLogsResponse { + return &apiv1.GetActionLogsResponse{ + CountRemaining: 0, + PageSize: page.size, + TotalCount: page.total, + StartOffset: page.start, + } +} + +// calculateReversedIndices computes the reversed indices for newest-first pagination. +func calculateReversedIndices(page pageInfo, filteredLen int) (int64, int64) { + startIdx := page.total - page.end + endIdx := page.total - page.start + if startIdx < 0 { + startIdx = 0 + } + if endIdx > int64(filteredLen) { + endIdx = int64(filteredLen) + } + return startIdx, endIdx +} + +// buildActionLogsResponse builds the response with paginated log entries. +func (api *oliveTinAPI) buildActionLogsResponse(filtered []*executor.InternalLogEntry, page pageInfo, user *acl.AuthenticatedUser) *apiv1.GetActionLogsResponse { + startIdx, endIdx := calculateReversedIndices(page, len(filtered)) + ret := &apiv1.GetActionLogsResponse{} + for _, le := range filtered[startIdx:endIdx] { + ret.Logs = append(ret.Logs, api.internalLogEntryToPb(le, user)) + } + ret.CountRemaining = page.start + ret.PageSize = page.size + ret.TotalCount = page.total + ret.StartOffset = page.start + return ret +} + func (api *oliveTinAPI) GetActionLogs(ctx ctx.Context, req *connect.Request[apiv1.GetActionLogsRequest]) (*connect.Response[apiv1.GetActionLogsResponse], error) { user := acl.UserFromContext(ctx, req, api.cfg) @@ -471,43 +518,22 @@ func (api *oliveTinAPI) GetActionLogs(ctx ctx.Context, req *connect.Request[apiv return nil, err } - ret := &apiv1.GetActionLogsResponse{} filtered := api.filterLogsByACL(api.executor.GetLogsByActionId(req.Msg.ActionId), user) page := paginate(int64(len(filtered)), api.cfg.LogHistoryPageSize, req.Msg.StartOffset) if page.empty { - ret.CountRemaining = 0 - ret.PageSize = page.size - ret.TotalCount = page.total - ret.StartOffset = page.start - return connect.NewResponse(ret), nil + return connect.NewResponse(buildEmptyPageResponse(page)), nil } - // Newest-first slicing: compute reversed indices - startIdx := page.total - page.end - endIdx := page.total - page.start - if startIdx < 0 { - startIdx = 0 - } - if endIdx > int64(len(filtered)) { - endIdx = int64(len(filtered)) - } - for _, le := range filtered[startIdx:endIdx] { - ret.Logs = append(ret.Logs, api.internalLogEntryToPb(le, user)) - } - // Entries older than the returned newest page - ret.CountRemaining = page.start - ret.PageSize = page.size - ret.TotalCount = page.total - ret.StartOffset = page.start - return connect.NewResponse(ret), nil + + return connect.NewResponse(api.buildActionLogsResponse(filtered, page, user)), nil } func (api *oliveTinAPI) pbLogsFiltered(entries []*executor.InternalLogEntry, user *acl.AuthenticatedUser) []*apiv1.LogEntry { out := make([]*apiv1.LogEntry, 0, len(entries)) for _, e := range entries { - if e == nil || e.Binding == nil || e.Binding.Action == nil { + if !isValidLogEntry(e) { continue } - if acl.IsAllowedLogs(api.cfg, user, e.Binding.Action) { + if api.isLogEntryAllowed(e, user) { out = append(out, api.internalLogEntryToPb(e, user)) } } @@ -517,10 +543,10 @@ func (api *oliveTinAPI) pbLogsFiltered(entries []*executor.InternalLogEntry, use func (api *oliveTinAPI) filterLogsByACL(entries []*executor.InternalLogEntry, user *acl.AuthenticatedUser) []*executor.InternalLogEntry { filtered := make([]*executor.InternalLogEntry, 0, len(entries)) for _, e := range entries { - if e == nil || e.Binding == nil || e.Binding.Action == nil { + if !isValidLogEntry(e) { continue } - if acl.IsAllowedLogs(api.cfg, user, e.Binding.Action) { + if api.isLogEntryAllowed(e, user) { filtered = append(filtered, e) } } diff --git a/service/internal/config/config_reloader.go b/service/internal/config/config_reloader.go index 9909f02..ddbe9e4 100644 --- a/service/internal/config/config_reloader.go +++ b/service/internal/config/config_reloader.go @@ -72,16 +72,29 @@ func afterLoadFinalize(cfg *Config, configPath string) { } } +// buildIncludePath constructs the full path to the include directory. +func buildIncludePath(k *koanf.Koanf, baseConfigPath string) string { + relativeIncludePath := k.String("include") + return filepath.Join(filepath.Dir(baseConfigPath), relativeIncludePath) +} + +// loadAndMergeYamlFiles loads and merges all YAML files from the include directory. +func loadAndMergeYamlFiles(k *koanf.Koanf, includePath string, yamlFiles []string) { + sort.Strings(yamlFiles) + for _, filename := range yamlFiles { + loadAndMergeIncludedFile(k, includePath, filename) + } + log.Infof("Finished loading %d included config file(s)", len(yamlFiles)) +} + // loadIncludedConfigsFromDir loads configuration files from an include directory and merges them func loadIncludedConfigsFromDir(k *koanf.Koanf, baseConfigPath string) { relativeIncludePath := k.String("include") - if relativeIncludePath == "" { return } - includePath := filepath.Join(filepath.Dir(baseConfigPath), relativeIncludePath) - + includePath := buildIncludePath(k, baseConfigPath) log.WithFields(log.Fields{ "includePath": includePath, }).Infof("Loading included configs from dir") @@ -91,42 +104,58 @@ func loadIncludedConfigsFromDir(k *koanf.Koanf, baseConfigPath string) { return } - sort.Strings(yamlFiles) - for _, filename := range yamlFiles { - loadAndMergeIncludedFile(k, includePath, filename) - } - - log.Infof("Finished loading %d included config file(s)", len(yamlFiles)) + loadAndMergeYamlFiles(k, includePath, yamlFiles) } -func listYamlFiles(includePath string) ([]string, bool) { +// validateIncludeDirectory checks if the given path exists and is a directory. +func validateIncludeDirectory(includePath string) bool { dirInfo, err := os.Stat(includePath) if err != nil { log.Warnf("Include directory not found: %s", includePath) - return nil, false + return false } if !dirInfo.IsDir() { log.Warnf("Include path is not a directory: %s", includePath) - return nil, false - } - entries, err := os.ReadDir(includePath) - if err != nil { - log.Errorf("Error reading include directory: %v", err) - return nil, false + return false } + return true +} + +// isYamlFile checks if a filename has a YAML extension. +func isYamlFile(name string) bool { + return strings.HasSuffix(name, ".yml") || strings.HasSuffix(name, ".yaml") +} + +// filterYamlFilesFromEntries extracts YAML file names from directory entries. +func filterYamlFilesFromEntries(entries []os.DirEntry) []string { var yamlFiles []string for _, entry := range entries { if entry.IsDir() { continue } - name := entry.Name() - if strings.HasSuffix(name, ".yml") || strings.HasSuffix(name, ".yaml") { - yamlFiles = append(yamlFiles, name) + if isYamlFile(entry.Name()) { + yamlFiles = append(yamlFiles, entry.Name()) } } + return yamlFiles +} + +func listYamlFiles(includePath string) ([]string, bool) { + if !validateIncludeDirectory(includePath) { + return nil, false + } + + entries, err := os.ReadDir(includePath) + if err != nil { + log.Errorf("Error reading include directory: %v", err) + return nil, false + } + + yamlFiles := filterYamlFilesFromEntries(entries) if len(yamlFiles) == 0 { log.Infof("No YAML files found in include directory: %s", includePath) } + return yamlFiles, true } @@ -143,27 +172,30 @@ func loadAndMergeIncludedFile(k *koanf.Koanf, includePath, filename string) { }).Info("Successfully loaded included config file") } -func mergeFunc(src map[string]interface{}, dest map[string]interface{}) error { - // Handle actions merging - koanf provides []interface{} not []*Action - // Merge src (new) into dest (existing) by appending src's actions to dest's actions - if srcActions, ok := src["actions"]; ok { - if destActions, ok := dest["actions"]; ok { - // Both have actions - append src to dest - srcSlice, ok1 := srcActions.([]interface{}) - destSlice, ok2 := destActions.([]interface{}) - if ok1 && ok2 { - dest["actions"] = append(destSlice, srcSlice...) - } else { - // Fallback: if types don't match, just use src - dest["actions"] = srcActions - } - } else { - // dest doesn't have actions, so use src's actions - dest["actions"] = srcActions - } +// mergeActionsWhenBothExist merges actions when both src and dest have actions. +func mergeActionsWhenBothExist(srcActions interface{}, destActions interface{}, dest map[string]interface{}) { + srcSlice, ok1 := srcActions.([]interface{}) + destSlice, ok2 := destActions.([]interface{}) + if ok1 && ok2 { + dest["actions"] = append(destSlice, srcSlice...) + } else { + dest["actions"] = srcActions } - // If src doesn't have actions, leave dest unchanged +} +// mergeActionsFromSource merges actions from source into destination. +func mergeActionsFromSource(srcActions interface{}, dest map[string]interface{}) { + if destActions, ok := dest["actions"]; ok { + mergeActionsWhenBothExist(srcActions, destActions, dest) + } else { + dest["actions"] = srcActions + } +} + +func mergeFunc(src map[string]interface{}, dest map[string]interface{}) error { + if srcActions, ok := src["actions"]; ok { + mergeActionsFromSource(srcActions, dest) + } return nil } diff --git a/service/internal/config/config_reloader_test.go b/service/internal/config/config_reloader_test.go index 24ed3e3..6e019d1 100644 --- a/service/internal/config/config_reloader_test.go +++ b/service/internal/config/config_reloader_test.go @@ -90,63 +90,44 @@ var envConfigTests = []struct { } func TestEnvInConfig(t *testing.T) { - for _, tt := range envConfigTests { - cfg := DefaultConfig() - setIfNotEmpty("INPUT", tt.input) - processed := processYamlWithEnv(tt.yaml) - k, err := loadKoanf(processed) - if err != nil { - t.Errorf("Error loading YAML: %v", err) - continue - } - if err := k.Unmarshal(".", cfg); err != nil { - t.Errorf("Error unmarshalling config: %v", err) - continue - } - manualAssigns(k, cfg) - field := tt.selector(cfg) - assert.Equal(t, tt.output, field, "Unmarshaled config field doesn't match expected value: env=\"%s\"", tt.input) - os.Unsetenv("INPUT") - } + for _, tt := range envConfigTests { + cfg := DefaultConfig() + setIfNotEmpty("INPUT", tt.input) + processed := processYamlWithEnv(tt.yaml) + k, err := loadKoanf(processed) + if err != nil { + t.Errorf("Error loading YAML: %v", err) + continue + } + if err := k.Unmarshal(".", cfg); err != nil { + t.Errorf("Error unmarshalling config: %v", err) + continue + } + field := tt.selector(cfg) + assert.Equal(t, tt.output, field, "Unmarshaled config field doesn't match expected value: env=\"%s\"", tt.input) + os.Unsetenv("INPUT") + } } func setIfNotEmpty(key, val string) { - if val != "" { - os.Setenv(key, val) - } + if val != "" { + os.Setenv(key, val) + } } func processYamlWithEnv(content string) string { - return envRegex.ReplaceAllStringFunc(content, func(match string) string { - submatches := envRegex.FindStringSubmatch(match) - key := submatches[1] - val, _ := os.LookupEnv(key) - return val - }) + return envRegex.ReplaceAllStringFunc(content, func(match string) string { + submatches := envRegex.FindStringSubmatch(match) + key := submatches[1] + val, _ := os.LookupEnv(key) + return val + }) } func loadKoanf(processed string) (*koanf.Koanf, error) { - k := koanf.New(".") - if err := k.Load(rawbytes.Provider([]byte(processed)), yaml.Parser()); err != nil { - return nil, err - } - return k, nil -} - -func manualAssigns(k *koanf.Koanf, cfg *Config) { - if k.Exists("PageTitle") { - cfg.PageTitle = k.String("PageTitle") - } - if k.Exists("CheckForUpdates") { - cfg.CheckForUpdates = k.Bool("CheckForUpdates") - } - if k.Exists("LogHistoryPageSize") { - cfg.LogHistoryPageSize = k.Int64("LogHistoryPageSize") - } - if k.Exists("actions") { - var actions []*Action - if err := k.Unmarshal("actions", &actions); err == nil { - cfg.Actions = actions - } - } + k := koanf.New(".") + if err := k.Load(rawbytes.Provider([]byte(processed)), yaml.Parser()); err != nil { + return nil, err + } + return k, nil } diff --git a/service/internal/executor/arguments.go b/service/internal/executor/arguments.go index debba3f..960014d 100644 --- a/service/internal/executor/arguments.go +++ b/service/internal/executor/arguments.go @@ -42,13 +42,8 @@ func parseCommandForReplacements(shellCommand string, values map[string]string, return shellCommand, nil } -func parseActionExec(values map[string]string, action *config.Action, entity *entities.Entity) ([]string, error) { - if action == nil { - return nil, fmt.Errorf("action is nil") - } - if err := validateArguments(values, action); err != nil { - return nil, err - } +// parseExecArray parses all exec arguments in the action. +func parseExecArray(action *config.Action, values map[string]string, entity *entities.Entity) ([]string, error) { parsed := make([]string, len(action.Exec)) for i, a := range action.Exec { out, err := parseSingleExec(a, values, entity) @@ -57,6 +52,20 @@ func parseActionExec(values map[string]string, action *config.Action, entity *en } parsed[i] = out } + return parsed, nil +} + +func parseActionExec(values map[string]string, action *config.Action, entity *entities.Entity) ([]string, error) { + if action == nil { + return nil, fmt.Errorf("action is nil") + } + if err := validateArguments(values, action); err != nil { + return nil, err + } + parsed, err := parseExecArray(action, values, entity) + if err != nil { + return nil, err + } logParsedExec(action, parsed, values) return parsed, nil } diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index d0d184f..e539c6c 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -224,24 +224,39 @@ func (e *Executor) GetLogTrackingIds(startOffset int64, pageCount int64) ([]*Int return trackingIds, pagingResult } -// GetLogTrackingIdsACL returns logs filtered by ACL visibility for the user and -// paginated correctly based on the filtered set. -func (e *Executor) GetLogTrackingIdsACL(cfg *config.Config, user *acl.AuthenticatedUser, startOffset int64, pageCount int64) ([]*InternalLogEntry, *PagingResult) { - // Build filtered list in reverse-chronological order (matching GetLogTrackingIds) +// isValidLogEntryForACL checks if a log entry has all required fields for ACL checking. +func isValidLogEntryForACL(entry *InternalLogEntry) bool { + return entry != nil && entry.Binding != nil && entry.Binding.Action != nil +} + +// isLogEntryAllowedByACL checks if a log entry is allowed to be viewed by the user. +func isLogEntryAllowedByACL(cfg *config.Config, user *acl.AuthenticatedUser, entry *InternalLogEntry) bool { + return acl.IsAllowedLogs(cfg, user, entry.Binding.Action) +} + +// filterLogsByACL builds a filtered list of logs in reverse-chronological order +// that are visible to the user based on ACL rules. +func (e *Executor) filterLogsByACL(cfg *config.Config, user *acl.AuthenticatedUser) []*InternalLogEntry { filtered := make([]*InternalLogEntry, 0) e.logmutex.RLock() for i := len(e.logsTrackingIdsByDate) - 1; i >= 0; i-- { entry := e.logs[e.logsTrackingIdsByDate[i]] - if entry == nil || entry.Binding == nil || entry.Binding.Action == nil { + if !isValidLogEntryForACL(entry) { continue } - if acl.IsAllowedLogs(cfg, user, entry.Binding.Action) { + if isLogEntryAllowedByACL(cfg, user, entry) { filtered = append(filtered, entry) } } e.logmutex.RUnlock() + return filtered +} + +// paginateFilteredLogs applies pagination to a filtered list of logs and returns +// the paginated results along with pagination metadata. +func paginateFilteredLogs(filtered []*InternalLogEntry, startOffset int64, pageCount int64) ([]*InternalLogEntry, *PagingResult) { total := int64(len(filtered)) paging := &PagingResult{PageSize: pageCount, TotalCount: total, StartOffset: startOffset} @@ -250,13 +265,10 @@ func (e *Executor) GetLogTrackingIdsACL(cfg *config.Config, user *acl.Authentica return []*InternalLogEntry{}, paging } - // Compute start/end indices using the same semantics as GetLogTrackingIds, - // but over the filtered slice startIndex := getPagingStartIndex(startOffset, total) pageCount = min(total, pageCount) endIndex := max(0, (startIndex-pageCount)+1) - // Slice is inclusive of both ends in original logic, so iterate and collect out := make([]*InternalLogEntry, 0, pageCount) for i := endIndex; i <= startIndex && i < int64(len(filtered)); i++ { out = append(out, filtered[i]) @@ -266,6 +278,13 @@ func (e *Executor) GetLogTrackingIdsACL(cfg *config.Config, user *acl.Authentica return out, paging } +// GetLogTrackingIdsACL returns logs filtered by ACL visibility for the user and +// paginated correctly based on the filtered set. +func (e *Executor) GetLogTrackingIdsACL(cfg *config.Config, user *acl.AuthenticatedUser, startOffset int64, pageCount int64) ([]*InternalLogEntry, *PagingResult) { + filtered := e.filterLogsByACL(cfg, user) + return paginateFilteredLogs(filtered, startOffset, pageCount) +} + func (e *Executor) GetLog(trackingID string) (*InternalLogEntry, bool) { e.logmutex.RLock()