Refactor flags in tracee-rules

[grantseltzer]

This addresses some of the points in #499. In particular:

1. Removes the --trace-file, and --stdin-as flags.
2. Adds a new flag called --input-trace that allows the user to specify the input file and input format for events to tracee-rules.

Still need to address:

1. Need to add unit tests for the above parsing logic
2. What should the default options be?
3. Update tracee-rules UX to align with tracee-ebpf #499 (comment)

Signed-off-by: grantseltzer grantseltzer@gmail.com

[grantseltzer]

Thoughts on default options being reading gob from stdin?

Also for the eof option, should this mean cut the stream/execution once an EOF is reached in the input file?

[itaysk]

added some small comments, but the big one is to properly handle EOT flag. I've commented on the issue, let's first make sure you understand the requirement first (this is crucial to how tracee-ebpf->rules work)

[grantseltzer]

@itaysk I addressed your feedback, can you also verify my usage of tracee.Event vs. types.Event. I had to rebase and solve the conflict so just want to make sure I got the idea right of where to use each.

[grantseltzer]

I believe that based on the discussion in #508 this is ready for further review and the final question I have is what the default behavior should be for tracee-rules. I think it should be reading gob format from standard input.

[itaysk]

@grantseltzer the code in this PR doesn't implement the revised logic suggested in #508 , did you mean to say that it is?

regarding the defaults, if you want to include #508 in this PR, then:

--input-tracee file:{/path/to/file,stdin} (default: stdin)
--input-tracee format:{gob/json}

[grantseltzer]

@itaysk I figured I would make that a separate PR but certainly can into this one.

[grantseltzer]

I'm actually going to open it as a second PR, the two things are related but it's a different part of the repo.

[itaysk]

I figured I would make that a separate PR but certainly can into this one.

no that's fine, I just understood it this way because you mentioned the discussion.

so now, for gob you read until EOT but for JSON you don't. I'm not sure this is what we wanted, but we can let it slide because that's changing soon anyway

[simar7]

nit: Consider replacing with a switch statement.

[grantseltzer]

If it's all the same to you, I think this looks cleaner than this:

	switch opts.inputFormat {
	case jsonInputFormat:
		return setupTraceeJSONInputSource(opts)
	case gobInputFormat:
		return setupTraceeGobInputSource(opts)
	default:
		return nil, errors.New("could not set up input source")

	}

[simar7]

Is it possible for the input file to be closed when we get here? If that happens we will be stuck in a loop forever.

[simar7]

(I realize it's not a change of logic here but it's just me looking at this with a fresh set of eyes)

[grantseltzer]

Hm, that is a good point but i'm not sure how it would?

[simar7]

Is this function tested somewhere else? Maybe we could write a simple test like:

func Test_setupTraceeGobInputSource(t *testing.T) {
	f, err := ioutil.TempFile("", "Test_setupTraceeGobInputSource-*")
	require.NoError(t, err)
	defer func() {
		_ = f.Close()
		_ = os.RemoveAll(f.Name())
	}()

	// write to file
	e := gob.NewEncoder(f)
	event := tracee.Event{
		EventName: "foo",
	}
	require.NoError(t, e.Encode(event))
	f.Seek(0, io.SeekStart)

	opts := traceeInputOptions{
		inputFile:   f,
		inputFormat: gobInputFormat,
	}

	res, err := setupTraceeGobInputSource(&opts)

	require.NoError(t, err)
	assert.Equal(t, "foo", (<-res).(tracee.Event).EventName)
}

[grantseltzer]

In the interest of unblocking Itay I'm going to include this in a follow up PR, but yes you do make a good point, testing these like how you suggested would be a big win.

[grantseltzer]

Issue created: #517

[itaysk]

please also update the readme accordingly and squash before you merge