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

Add addUsernameFromFilePathStep to support moving chrome_login_data_emails to KATC #1832

Conversation

RebeccaMahany
Copy link
Contributor

To support moving the kolide_chrome_login_data_emails table to KATC, we need to be able to add a username column that extracts the username from the source file path. This PR adds a transform step that does that.

@RebeccaMahany RebeccaMahany added the component:table Table Changes label Aug 13, 2024
}

const (
snappyDecodeTransformStep = "snappy"
deserializeFirefoxTransformStep = "deserialize_firefox"
deserializeChromeTransformStep = "deserialize_chrome"
camelToSnakeTransformStep = "camel_to_snake"
addUsernameFromFilePathStep = "add_username_from_filepath"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm somewhat unclear if we want this. The alternate pattern is to join using the home directory instead of username

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source path is already included in the KATC table data, so I imagined we could do something like that with the preexisting data instead! I added this because I figured it would be better for the katc_chrome_login_data_emails to provide the exact same columns as the kolide_ table does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to keep the same table semantics.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the assumptions in homeDirLocations have edges cases that will break unexpectedly :|

Copy link
Contributor Author

@RebeccaMahany RebeccaMahany Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edges cases that will break unexpectedly :|

In that case, the row will still be returned, just without the username field included! And we'd still have the path column available.

I think my preference is to keep this for parity with the old table, just in case we do want it. But if you feel strongly about getting rid of the username column, I'm happy to close this PR too!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch is we should drop it. I'm not quite sure why. Unless we cannot avoid it, I'd rather not return potentially wrong data.

@RebeccaMahany
Copy link
Contributor Author

Decided we don't want the username column, so I'm closing this PR

@RebeccaMahany RebeccaMahany deleted the becca/katc_chrome_login_data_emails branch August 14, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:table Table Changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants