EX-1850 - Implement Report query and deletion #2304

johnsonw · 2020-10-02T19:55:35Z

Implement Report query and deletion in graphql

Signed-off-by: johnsonw wjohnson@whamcloud.com

This change is

johnsonw · 2020-10-02T19:57:10Z

[root@adm report]# touch file1.txt
[root@adm report]# ls
file1.txt
[root@adm report]# ls
[root@adm report]#

iml-api/src/graphql.rs

johnsonw · 2020-10-05T16:49:03Z

Docker test:

Displaying files:

[root@iml-api report]# ls
[root@iml-api report]# ls
expiring_fids-fs-32621d4c-9b38-4d22-8d4a-9a45d96f5667.txt

Deleting a file:

[root@iml-api report]# ls
expiring_fids-fs-52ee437c-646a-402a-9df3-e90810a77a68.txt
[root@iml-api report]# ls
[root@iml-api report]#

Implement Report query and deletion in graphql Signed-off-by: johnsonw <wjohnson@whamcloud.com>

Signed-off-by: Joe Grund <jgrund@whamcloud.io>

johnsonw · 2020-10-05T16:56:47Z

Trying to delete a file maliciously:

…ndpoints to view and remove reports under /var/spool/iml/report. Signed-off-by: johnsonw <wjohnson@whamcloud.com>

Signed-off-by: johnsonw <wjohnson@whamcloud.com>

mkpankov

Reviewed 2 of 2 files at r4.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @johnsonw)

johnsonw · 2020-10-06T13:48:59Z

Integration tests passed here: https://build.whamcloud.com/view/IML/job/manager-for-lustre/2489/

johnsonw · 2020-10-06T13:50:38Z

Also, I've updated the query to return both the filesize and modified time. The data returned now looks like this:

{
  "data": {
    "stratagemReports": [
      {
        "filename": "/var/spool/iml/report/test.txt",
        "modifyTime": "2020-10-05T21:51:57.887528595+00:00",
        "size": 0
      }
    ]
  }
}

johnsonw · 2020-10-06T13:52:45Z

The CodeQL failure is unrelated and I see it in other patches as well.

jgrund · 2020-10-06T14:02:07Z

Does this now print the whole file path or just the name?

johnsonw · 2020-10-06T14:04:21Z

It's printing the whole file path. I thought it might be useful to show the whole path in case they weren't aware of the report being under /var/spool/iml. However, we could always print a note above indicating where the file lives and then just show the filename. What are your thoughts?

jgrund · 2020-10-06T14:06:37Z

Probably just the filename. The reports can be downloaded at /report/<filename>, so it's probably not important to know the full path. In addition, if we are running in containers, that path will exist inside a container only and will be confusing to users that it's not on the host.

johnsonw · 2020-10-06T14:14:57Z

Sure, makes sense. I will update it to be the filename only.

…te path. Signed-off-by: johnsonw <wjohnson@whamcloud.com>

johnsonw · 2020-10-06T16:17:38Z

Here is an updated screenshot. I created three files under /var/spool/iml/report. Notice that only the filename is now displayed:
test1.txt = 1Kib
test2.txt = 1Mib
test3.txt = 1Gib

ip1981 · 2020-10-06T16:52:26Z

iml-api/src/graphql.rs

+ let path = tokio::fs::canonicalize(report_base.join(filename)).await?;
+
+ if !path.starts_with(report_base) {
+ return Err(FieldError::new("Invalid path", Value::null()));


nit:

Suggested change

return Err(FieldError::new("Invalid path", Value::null()));

return Err(FieldError::new("Invalid filename", Value::null()));

ip1981 · 2020-10-06T17:03:32Z

iml-api/src/graphql.rs

+ .await?
+ .into_iter()
+ .map(|filename| {
+ fs::canonicalize(get_report_path().join(filename.clone()))


Could call let report_dir = get_report_path() just once at the beginning?

ip1981 · 2020-10-06T17:05:25Z

iml-api/src/graphql.rs

+ /// List completed Stratagem reports that currently reside on the manager node.
+ /// Note: All report names must be valid unicode.
+ async fn stratagem_reports(_context: &Context) -> juniper::FieldResult<Vec<StratagemReport>> {
+ let paths = tokio::fs::read_dir(get_report_path()).await?;


nit:

Suggested change

let paths = tokio::fs::read_dir(get_report_path()).await?;

let paths = fs::read_dir(get_report_path()).await?;

ip1981 · 2020-10-06T17:06:48Z

iml-api/src/graphql.rs

+ .await?
+ .into_iter()
+ .map(|filename| {
+ fs::canonicalize(get_report_path().join(filename.clone()))


Is clone() needed?

johnsonw added the enhancement label Oct 2, 2020

johnsonw self-assigned this Oct 2, 2020

jgrund reviewed Oct 2, 2020

View reviewed changes

iml-api/src/graphql.rs Outdated Show resolved Hide resolved

jgrund reviewed Oct 2, 2020

View reviewed changes

iml-api/src/graphql.rs Outdated Show resolved Hide resolved

johnsonw force-pushed the EX-1850 branch from 8a3302f to bba526c Compare October 2, 2020 19:58

johnsonw and others added 2 commits October 5, 2020 12:52

EX-1850 - Implement Report query and deletion

e4a5b90

Implement Report query and deletion in graphql Signed-off-by: johnsonw <wjohnson@whamcloud.com>

Updates

a310778

Signed-off-by: Joe Grund <jgrund@whamcloud.io>

johnsonw force-pushed the EX-1850 branch from 39303ae to 4d6e774 Compare October 5, 2020 16:53

johnsonw marked this pull request as ready for review October 5, 2020 16:58

jgrund self-requested a review October 5, 2020 17:45

johnsonw force-pushed the EX-1850 branch from 4d6e774 to 272fc94 Compare October 5, 2020 20:58

Add report volume to iml-api. This will allow the stratagem graphql e…

5c0253e

…ndpoints to view and remove reports under /var/spool/iml/report. Signed-off-by: johnsonw <wjohnson@whamcloud.com>

johnsonw force-pushed the EX-1850 branch from 272fc94 to 5c0253e Compare October 5, 2020 20:59

- Remove create time from StratagemReport type as it's not on linux

aef462a

Signed-off-by: johnsonw <wjohnson@whamcloud.com>

mkpankov previously approved these changes Oct 6, 2020

View reviewed changes

Use only the filename in the report filename property, not the absolu…

bb834a3

…te path. Signed-off-by: johnsonw <wjohnson@whamcloud.com>

johnsonw dismissed mkpankov’s stale review via bb834a3 October 6, 2020 15:39

johnsonw requested a review from mkpankov October 6, 2020 16:18

ip1981 reviewed Oct 6, 2020

View reviewed changes

jgrund approved these changes Oct 6, 2020

View reviewed changes

ip1981 reviewed Oct 6, 2020

View reviewed changes

jgrund merged commit 3a7da93 into master Oct 6, 2020

jgrund deleted the EX-1850 branch October 6, 2020 17:03

ip1981 reviewed Oct 6, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EX-1850 - Implement Report query and deletion #2304

EX-1850 - Implement Report query and deletion #2304

johnsonw commented Oct 2, 2020 •

edited by jgrund

Loading

johnsonw commented Oct 2, 2020

johnsonw commented Oct 5, 2020

johnsonw commented Oct 5, 2020

mkpankov left a comment

johnsonw commented Oct 6, 2020

johnsonw commented Oct 6, 2020

johnsonw commented Oct 6, 2020

jgrund commented Oct 6, 2020

johnsonw commented Oct 6, 2020 •

edited

Loading

jgrund commented Oct 6, 2020 •

edited

Loading

johnsonw commented Oct 6, 2020

johnsonw commented Oct 6, 2020

ip1981 Oct 6, 2020

ip1981 Oct 6, 2020 •

edited

Loading

ip1981 Oct 6, 2020

ip1981 Oct 6, 2020

	return Err(FieldError::new("Invalid path", Value::null()));
	return Err(FieldError::new("Invalid filename", Value::null()));

	let paths = tokio::fs::read_dir(get_report_path()).await?;
	let paths = fs::read_dir(get_report_path()).await?;

EX-1850 - Implement Report query and deletion #2304

EX-1850 - Implement Report query and deletion #2304

Conversation

johnsonw commented Oct 2, 2020 • edited by jgrund Loading

johnsonw commented Oct 2, 2020

johnsonw commented Oct 5, 2020

johnsonw commented Oct 5, 2020

mkpankov left a comment

Choose a reason for hiding this comment

johnsonw commented Oct 6, 2020

johnsonw commented Oct 6, 2020

johnsonw commented Oct 6, 2020

jgrund commented Oct 6, 2020

johnsonw commented Oct 6, 2020 • edited Loading

jgrund commented Oct 6, 2020 • edited Loading

johnsonw commented Oct 6, 2020

johnsonw commented Oct 6, 2020

ip1981 Oct 6, 2020

Choose a reason for hiding this comment

ip1981 Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

ip1981 Oct 6, 2020

Choose a reason for hiding this comment

ip1981 Oct 6, 2020

Choose a reason for hiding this comment

johnsonw commented Oct 2, 2020 •

edited by jgrund

Loading

johnsonw commented Oct 6, 2020 •

edited

Loading

jgrund commented Oct 6, 2020 •

edited

Loading

ip1981 Oct 6, 2020 •

edited

Loading