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

Changes needed for the integration of ABRT in QE workflows #388

Merged
merged 10 commits into from
Oct 27, 2015

Conversation

jfilak
Copy link
Contributor

@jfilak jfilak commented Oct 20, 2015

We need to be able to attach the URL of the uploaded problem data to the relevant micro-report. Here are the problems addressed by this pull request.

  1. reporter-upload does not save its report results in reported_to file
  2. reporter-ureport does not know 'url' micro-report attachment type [URL attachment saving faf#467]

How should be this pull request used in the integration efforts:

EVENT=notify
  reporter-ureport
  EC=$?

  # See  EXIT_STOP_EVENT_RUN in /usr/include/libreport/internal_libreport.h
  if [ $EC -eq 70 ]; then
   echo "Already reported problem:"
    cat reported_to
    exit 0
  fi

  if [ $EC -ne 0 ]; then
    exit $EC
  fi

  reporter-upload -u ftp://qe.example.org/abrt-reports || exit 1
  reporter-ureport -r "upload" -L "URL" -T "url" || exit 1

Jakub Filak added 2 commits October 19, 2015 15:02
Introduce dd_create_archive() function.

I added the argument 'flags' in order to avoid the need to introduce
dd_create_archive_ext() function in the future. I am sure will use this
flag in the future (e.g. Encrypted).

Signed-off-by: Jakub Filak <[email protected]>
Use the test function instead of own untested one.

Signed-off-by: Jakub Filak <[email protected]>
@jfilak jfilak assigned jfilak and mhabrnal and unassigned jfilak Oct 20, 2015
Jakub Filak added 3 commits October 20, 2015 12:44
Make sure we can convert data back and forth without losing information.
The introduced function complements iso_date_string().

Signed-off-by: Jakub Filak <[email protected]>
Use this function to ensure consistent reported_to formatting.

Signed-off-by: Jakub Filak <[email protected]>
This commit should make the code less fragile. There is no need for this
commit, however I plan to make the '.label' member configurable through
environment variables.

Signed-off-by: Jakub Filak <[email protected]>
@jfilak jfilak force-pushed the wip_jfilak_base_os_qa branch 2 times, most recently from 151cc52 to e75c504 Compare October 20, 2015 11:33
@jfilak
Copy link
Contributor Author

jfilak commented Oct 20, 2015

ABRT integration test case (not pushed yet):
abrt/abrt@3c3544b

@jfilak
Copy link
Contributor Author

jfilak commented Oct 20, 2015

Do not merge this pull request. The lines added to reported_to file can contain usernames and passwords:

upload: URL=scp://root:redhat@localhost/tmp/tmp.RaYnVplNNv/target/problem_dir.tar.gz

I need to either ask curl to return URL without credentials or remove everything between :// and @.

@jfilak jfilak force-pushed the wip_jfilak_base_os_qa branch from e75c504 to 17d9b75 Compare October 20, 2015 13:22
@jfilak
Copy link
Contributor Author

jfilak commented Oct 20, 2015

I've fixed the problem with credentials and added the following commit to abrt:
abrt/abrt@4b307cc

Jakub Filak added 4 commits October 21, 2015 16:45
The function expects a valid URL.

Signed-off-by: Jakub Filak <[email protected]>
All clients should work with URLs without userinfo. This commit will
ensure that.

This commit also changes log messages from:

    Sending /var/tmp/problem_dir.tar.gz to scp://localhost/tmp/tmp.x5WVgpgUsY/target/
    Error while uploading: 'curl_easy_perform: Login denied'
    Please enter user name for 'scp://localhost/tmp/tmp.x5WVgpgUsY/target/': root
    Please enter password for 'root':
    Sending /var/tmp/problem_dir.tar.gz to scp://localhost/tmp/tmp.x5WVgpgUsY/target/
    Successfully sent /var/tmp/problem_dir.tar.gz to scp://localhost/tmp/tmp.x5WVgpgUsY/target/

to:

    Sending /var/tmp/problem_dir.tar.gz to scp://localhost
    Error while uploading: 'curl_easy_perform: Login denied'
    Please enter user name for 'scp://localhost': root
    Please enter password for 'scp://root@localhost':
    Sending /var/tmp/problem_dir.tar.gz to scp://localhost
    Successfully created scp://localhost/tmp/tmp.x5WVgpgUsY/target/problem_dir.tar.gz

Signed-off-by: Jakub Filak <[email protected]>
All other plugins do so. We also need this commit to be able to attach
URLs to uploaded dump directories to uReport on FAF servers.

Signed-off-by: Jakub Filak <[email protected]>
Signed-off-by: Jakub Filak <[email protected]>
@jfilak jfilak force-pushed the wip_jfilak_base_os_qa branch from 17d9b75 to 5b8a29f Compare October 21, 2015 14:49
@jfilak
Copy link
Contributor Author

jfilak commented Oct 21, 2015

This pull request can be reviewed and merged.

{
/* Load Password only if Username is configured, it doesn't make */
/* much sense to load Password without Username. */
state->password = getenv("Upload_Password");
Copy link
Contributor

Choose a reason for hiding this comment

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

What if user configures scp to use keys? There might be username set but no password is needed. Also using CURL to upload stuff via SCP is not good at all, would be much better to just use SCP so user can configure it as usual and not fiddle with CURL/abrt specific configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I will try to rework this part later on: #389

@jfilak jfilak force-pushed the wip_jfilak_base_os_qa branch from 5b8a29f to 99d45ca Compare October 22, 2015 06:35
@jfilak
Copy link
Contributor Author

jfilak commented Oct 22, 2015

I corrected the documentation issues found by @sorki

@sorki
Copy link
Contributor

sorki commented Oct 22, 2015

Good to go from my side, thanks.

@jfilak jfilak force-pushed the wip_jfilak_base_os_qa branch 2 times, most recently from c5b0dcc to 5b9d8bf Compare October 27, 2015 12:03
Whenever we introduce a new report type we need to teach
reporter-ureport to attach its report results to micro-reports.

This commit adds support for attaching arbitrary values, so we will not
need to modify reporter-ureport when a new reporter is added.

Two operating modes are being introduced.
The mode for attaching random values:
$ reporter-ureport -l "foo" -T "blah" -A

and the mode for attaching the report results
(data from reported_to file):
$ reporter-ureport -L "URL" -T "url" -r "upload" -A

'-v', '--value' is taken by '--verbose'.
'-d', '--data' is taken by '--directory'.
Hence I used '-l', '--value'.

'-t', '--type' is taken by '--auth'.
Hence I used '-T', '--type'.

Signed-off-by: Jakub Filak <[email protected]>
@jfilak jfilak force-pushed the wip_jfilak_base_os_qa branch from 5b9d8bf to ca9e85e Compare October 27, 2015 12:06
@mhabrnal
Copy link
Contributor

Looks good to me, thanks.

mhabrnal pushed a commit that referenced this pull request Oct 27, 2015
Changes needed for the integration of ABRT in QE workflows
@mhabrnal mhabrnal merged commit 2d34318 into master Oct 27, 2015
@jfilak jfilak deleted the wip_jfilak_base_os_qa branch October 30, 2015 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants