[Compass-ci] Re: [PATCH v7 compass-ci 2/6] container/mail-robot: answerback-email.rb

On Mon, Oct 19, 2020 at 02:57:28PM +0800, Luan Shengde wrote:

AssignUuid: read email content check email available check if base url in upstream-repos check if commit url valid check pub_key assign account generate uuid write account/uuid to es send uuid to user

Signed-off-by: Luan Shengde luanshengde2@huawei.com

container/mail-robot/answerback-email.rb | 229 +++++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100755 container/mail-robot/answerback-email.rb

diff --git a/container/mail-robot/answerback-email.rb b/container/mail-robot/answerback-email.rb new file mode 100755 index 0000000..66d13e4 --- /dev/null +++ b/container/mail-robot/answerback-email.rb @@ -0,0 +1,229 @@ +#!/usr/bin/env ruby +# SPDX-License-Identifier: MulanPSL-2.0+ +# Copyright (c) 2020 Huawei Technologies Co., Ltd. All rights reserved. +# frozen_string_literal: true

+require 'json' +require 'mail' +require_relative '/c/compass-ci/lib/es_client.rb'

That path should be improved.

+# Assign uuid/account +class AssignUuid

=> AssignAccount

• def initialize(mail_content)
• @mail_content = mail_content
• @send_mail_server = %x(/sbin/ip route |awk '/default/ {print $3}').chomp
• end
• def build_message(sum_infos)
• message = <<~EMAIL_MESSAGE

•  To: #{sum_infos['my_email']}
  

•  Subject: Account Ready
  

What account? This can be more clear.

[compass-ci] account ready

•  Dear #{sum_infos['my_name']}
  

append ','

•    Thank you for joining us.
  

•    You can use the following info to submit jobs:
  

•    First: add the info to ~/.config/compass-ci/defaults/{USER}.yaml
  

Missed $ in {USER}

First => 1)

the info => the below setup

•      uuid: #{sum_infos['my_uuid']}
  

•      lab: z9
  

Better give ready-to-run commands.

mkdir -p ~/.config/compass-ci/defaults/

cat >> ~/.config/compass-ci/defaults/$USER.yaml <<-EOF my_name: my_email: my_uuid: #{sum_infos['my_uuid']} lab: z9 EOF

•    Second: download lkp-tests and dependencies
  

•      git clone https://gitee.com_/wu_fengguang/lkp-tests.git
  

•      cd lkp-tests
  

•      make install
  

•      source {HOME}/.bashrc && source {HOME}/.bash_profile
  

Please make it clear the above is one-time-setup.

•    Third: submit job
  

•      reference: https://gitee.com/wu_fengguang/compass-ci/blob/master/doc/tutorial.md
  

•      how to write job yaml
  

•      submit jobs/netperf.yaml
  

•    lastly: add your own information and uuid to ~/.config/compass-ci/default/{USER}.yml
  

Faint, please merge this with (1).

•      my_name: {your name}
  

•      my_email: {your email address}
  

•      my_uuid: {applied uuid}
  

•    Repalce the fields {...} with your local environment variable.
  

•  regards
  

•  compass-ci
  

• EMAIL_MESSAGE
• return message
• end
• def build_err_message(email, message)
• message = <<~EMAIL_MESSAGE

•  To: #{email}
  

•  Subject: apply account failed
  

•  Dear #{email.split('@')[0]}
  

•    Your application for account failed with following errors
  

•      #{message}
  

•  regards
  

•  compass-ci
  

• EMAIL_MESSAGE
• end
• def send_mail(email_msg)
• %x(curl -XPOST "#{@send_mail_server}:#{ENV['S_M_PORT']}/send_mail_text" -d "#{email_msg}")
• end
• def url_addr(email)
• mail_body = @mail_content.part[0].body.decoded
• mail_content = mail_body.gsub(/=/, '').split(/\r|\n|=/).join
• url = %x(echo '#{mail_content}' |grep -o -E "https?://[^/]*/[^/]*/[^/]*/[^/]*/[0-9a-zA-Z]{40}").split(/\n/)[0]

Why use grep in ruby?

Looks you didn't check "my oss commit: " prefix.

• if url.nil?

•  message = 'No commit url found'
  

•  email_msg = build_err_message(email, message)
  

调用层级混乱了. 这个函数就只管parse url. 不管发错误邮件.

•  send_mail(email_msg)
  

•  raise message
  

• end
• return url
• end
• def check_email_available(email)
• url = url_addr(email).chomp
• base_url = url.gsub(%r{/[^/]*/[0-9a-zA-Z]{40}$}, '')
• upstream_dir = '/c/upstream-repos'
• check_url_in_upstream_repos(upstream_dir, base_url, email)
• check_url_commit(url, base_url, email)
• return url
• end
• def check_url_in_upstream_repos(upstream_dir, base_url, email)
• Dir.chdir(upstream_dir)
• match_out = %x(grep -rn #{base_url})
• if match_out.empty?

•  message = 'The url is not in upstream-repo list'
  

•  email_msg = build_err_message(email, message)
  

•  send_mail(email_msg)
  

•  raise StandardError, message, base_url
  

• end
• end
• def check_url_commit(url, base_url, email)
• hub_name = url.split('/')[2]
• # it requires authentication when execute curl to get the commit infomation
• # clone the repo and then validate the commit for the email address

Pleas use if/else here.

• check_gitee_commit(url, base_url, email) if hub_name.eql? 'gitee.com'
• check_non_gitee_commit(url, email) unless hub_name.eql? 'gitee.com'
• end
• def check_non_gitee_commit(url, email)
• url_fdback = %x(curl #{url})
• email_index = url_fdback.index email
• if email_index.nil?

•  message = 'Your commit url is not a valid commit url.'
  

•  email_msg = build_err_message(email, message)
  

•  send_mail(email_msg)
  

•  raise StandardError, message, url
  

• end
• end
• def check_gitee_commit(url, base_url, email)
• repo_info = {

•  'repos_dir' => '/repos_dir',
  

/repos_dir should be made a constant in the beginning of the script. With proper comments for its usage.

•  'repo_dir' => url.split('/')[-3],
  

•  'repo_url' => [base_url, 'git'].join('.'),
  

•  'repo_path' => File.join('/repos_dir', url.split('/')[-3]),
  

•  'commit_id' => url.split('/')[-1]
  

• }
• check_gitee_repo_commit(url, email, repo_info)
• end
• def check_gitee_repo_commit(url, email, repo_info)
• Dir.chdir repo_info['repos_dir']
• %x(/usr/bin/git clone #{repo_info['repo_url']})
• email_index = %x(/usr/bin/git -C #{repo_info['repo_dir']} show #{repo_info['commit_id']}).index email
• FileUtils.rm_rf repo_info['repo_dir']
• if email_index.nil?

Don't send email here.

One function, one thing.

•  message = 'Your commit url is not a valid commit url.'
  

•  email_msg = build_err_message(email, message)
  

•  send_mail(email_msg)
  

•  raise StandardError, message, url
  

• end
• end
• def pub_key_value(email)
• pub_key = @mail_content.part[1].body.decoded.split(/\r|\n/).join if @mail_content.part[1].filename == 'id_rsa.pub'

too long line

• unless pub_key

•  message = 'No pub_key found'
  

•  email_msg = build_err_message(email, message)
  

•  send_mail(email_msg)
  

•  raise message
  

• end
• return pub_key
• end
• def account_info(pub_key, email)

这里明显主要是调用assign_account 用 account_info 这种名词做函数名不合适.

• account_info_str = %x(curl -XGET "#{ENV['J_HOST']}:#{ENV['J_PORT']}/assign_account" -d "pub_key: #{pub_key}")
• account_info = JSON.parse account_info_str

Don't mix function name and variable name with one single name.

• if account_info['account'].empty?

•  message = 'No more available jumper account.'
  

•  email_msg = build_err_message(email, message)
  

•  send_mail(email_msg)
  

•  raise StandardError, message, account_info if account_info['account'].empty?
  

• end
• return account_info
• end
• def build_sum_infos
• email = @mail_content.from[0]
• my_name = @mail_content.From.unparsed_value.gsub(/ <[^<>]*>/, '')

• commit_url = check_email_available(email)

How come you use so different names for the var and func?

• pub_key = pub_key_value(email)
• acct_infos = account_info(pub_key, email)
• account_uuid = %x(uuidgen).chomp

avoid use-once variable

• sum_infos = {

sum_infos => apply_info or applicant_info

•  'my_email' => email,
  

•  'my_name' => my_name,
  

•  'my_commit_url' => commit_url,
  

•  'my_login_name' => acct_infos['account'],
  

•  'my_uuid' => account_uuid,
  

• }
• store_account_info(sum_infos)
• sum_infos['acct_infos'] = acct_infos
• sum_infos
• end
• def send_uuid(sum_infos)
• email_msg = build_message(sum_infos)
• send_mail(email_msg)
• end
• def send_account
• sum_infos = build_sum_infos
• send_uuid(sum_infos)
• end
• def store_account_info(sum_infos)
• es = ESClient.new(index: 'accounts')
• es.put_source_by_id(sum_infos['my_email'], sum_infos)
• end

+end

2.23.0