Repository: Azure/azure-cli

Issue:
Weird error on login - 'dict' object cannot be interpreted as an integer
### Environment summary

**Install Method:** How did you install the CLI? (e.g. pip, interactive script, apt-get, Docker, MSI, nightly)  
```
        rm -rf $HOME/.local/lib/python3.6
        mkdir $HOME/.local/lib/python3.6/site-packages
        pip install --no-cache-dir --user --upgrade --pre azure-cli --extra-index-url https://azureclinightly.blob.core.windows.net/packages
        pip install --user --upgrade --pre azure-cli-acr azure-cli-keyvault --extra-index-url https://azureclinightly.blob.core.windows.net/packages
```

**CLI Version:** What version of the CLI and modules are installed? (Use `az --version`)  
```
azure-cli (2.0.2+1.dev20170406)

acr (2.0.0+1.dev20170406)
acs (2.0.2+1.dev20170406)
appservice (0.1.2+1.dev20170406)
batch (2.0.0+1.dev20170406)
cloud (2.0.0+1.dev20170406)
component (2.0.0+1.dev20170406)
configure (2.0.2+1.dev20170406)
container (0.1.2+1.dev20170406)
core (2.0.2+1.dev20170406)
documentdb (0.1.2+1.dev20170406)
feedback (2.0.0+1.dev20170406)
find (0.1.2rc1+1.dev20170312)
iot (0.1.2+1.dev20170406)
keyvault (2.0.0+1.dev20170406)
lab (0.0.1+1.dev20170406)
monitor (0.0.1+1.dev20170406)
network (2.0.2+1.dev20170406)
nspkg (2.0.0+1.dev20170406)
profile (2.0.2+1.dev20170406)
redis (0.1.1b3+1.dev20170406)
resource (2.0.2+1.dev20170406)
role (2.0.1+1.dev20170406)
sql (2.0.0+1.dev20170406)
storage (2.0.2+1.dev20170406)
vm (2.0.2+1.dev20170406)

Python (Linux) 3.6.1 (default, Mar 27 2017, 00:27:06)
[GCC 6.3.1 20170306]
```

**OS Version:** What OS and version are you using?  
Arch Linux, up to date

**Shell Type:** What shell are you using? (e.g. bash, cmd.exe, Bash on Windows)  
`zsh`

---

### Description
```
$ az login --service-principal --username xxx --password xxx --tenant xxx
'dict' object cannot be interpreted as an integer
Traceback (most recent call last):
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/main.py", line 37, in main
    cmd_result = APPLICATION.execute(args)
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/core/application.py", line 157, in execute
    result = expanded_arg.func(params)
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 367, in _execute_command
    raise ex
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 362, in _execute_command
    result = op(client, **kwargs) if client else op(**kwargs)
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/command_modules/profile/custom.py", line 79, in login
    tenant)
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/core/_profile.py", line 135, in find_subscriptions_on_login
    tenant))
  File "/home/cole/.local/lib/python3.6/site-packages/azure/cli/core/_profile.py", line 500, in save_service_principal_cred
    self._service_principal_creds.pop(matched[0])
TypeError: 'dict' object cannot be interpreted as an integer
```

If I `rm -rf ~/.azure` first, it goes away. Maybe a bug in merging stuff together?

~~Also, weird though, since my script sets `AZURE_CONFIG_DIR` to a temp dir, so it should be like new every time...~~ I wasn't properly exporting it, when I do, I avoid this issue since each script run gets its own config directory.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=h1) Report
> Merging [#2848](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=desc) into [master](https://codecov.io/gh/Azure/azure-cli/commit/f6b3c18535cf9d1753213311cfe686a7d8f1295d?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/azure-cli/pull/2848/graphs/tree.svg?height=150&width=650&token=2pog0TKvF8&src=pr)](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #2848      +/-   ##
==========================================
+ Coverage   62.92%   62.94%   +0.01%     
==========================================
  Files         464      464              
  Lines       25899    25899              
  Branches     3943     3943              
==========================================
+ Hits        16298    16302       +4     
+ Misses       8567     8563       -4     
  Partials     1034     1034
```


| [Impacted Files](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/azure-cli-core/azure/cli/core/\_profile.py](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=tree#diff-c3JjL2F6dXJlLWNsaS1jb3JlL2F6dXJlL2NsaS9jb3JlL19wcm9maWxlLnB5) | `85.25% <100%> (+1.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=footer). Last update [f6b3c18...d27d58a](https://codecov.io/gh/Azure/azure-cli/pull/2848?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/.gitignore b/.gitignore
index fc479e986..7346003bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,3 +77,6 @@ test_results/
 
 # Code coverage
 .coverage
+
+# CI output
+artifacts/
\ No newline at end of file
diff --git a/Jenkinsfile b/Jenkinsfile
new file mode 100644
index 000000000..a6f8e4dd9
--- /dev/null
+++ b/Jenkinsfile
@@ -0,0 +1,29 @@
+pipeline {
+    agent none
+    triggers { pollSCM('H/3 * * * *') }
+    stages {
+        stage ('Build') {
+            agent any
+            steps {
+                sh 'pip install -U virtualenv'
+                sh 'python -m virtualenv --clear env'
+                sh './scripts/jenkins_build.sh'
+                sh './scripts/jenkins_archive.sh'
+            }
+            post {
+                always { deleteDir() }
+            }
+        } 
+        stage ('Performance-Test') {
+            agent { label 'perf-ubuntu-a0' }
+            steps {
+                sh 'pip install -U virtualenv'
+                sh 'python -m virtualenv --clear env'
+                sh './scripts/jenkins_perf.sh'
+            }
+            post {
+                always { deleteDir() }
+            }
+        }
+    }
+}
diff --git a/scripts/jenkins_archive.sh b/scripts/jenkins_archive.sh
new file mode 100755
index 000000000..4422c60bd
--- /dev/null
+++ b/scripts/jenkins_archive.sh
@@ -0,0 +1,24 @@
+#!/usr/bin/env bash
+
+# Archive build
+
+set -x
+
+if [ -z $BUILD_NUMBER ]; then
+    echo "Environment variable BUILD_NUMBER is missing."
+    exit 1
+fi
+
+export
+echo "build branch $BRANCH_NAME"
+
+version=$(printf '%.8d' $BUILD_NUMBER)
+echo "Version number: $version"
+
+if [ -d /var/build_share ]; then
+    echo 'Directory /var/build_share is found. The artifacts will be archived there.'
+    mkdir -p /var/build_share/$BRANCH_NAME/$version
+    cp -R ./artifacts/ /var/build_share/$BRANCH_NAME/$version
+else
+    echo 'Directory /var/build_share is not found. Exit without taking any actions.'
+fi
diff --git a/scripts/jenkins_build.sh b/scripts/jenkins_build.sh
new file mode 100755
index 000000000..b12e2e093
--- /dev/null
+++ b/scripts/jenkins_build.sh
@@ -0,0 +1,26 @@
+# Build packages in Jenkins server.
+# The script expects a virtualenv created under ./env folder as prerequisite
+
+set -x  # do not echo command to prevent accidentally expose secrets
+
+. ./env/bin/activate
+
+echo 'Build Azure CLI and its command modules '
+
+if [ -d ./artifacts ]; then
+    rm -rf ./artifacts
+fi
+
+mkdir -p ./artifacts/build
+artifacts=$(cd ./artifacts/build && pwd)
+
+working_dir=$(pwd)
+for setup_file in $(find src -name 'setup.py'); do
+    cd $(dirname $setup_file)
+    echo ""
+    echo "Components at $(pwd) is being built ..."
+    python setup.py sdist -d $artifacts bdist_wheel -d $artifacts
+    cd $working_dir
+done
+
+echo 'Build completed.'
\ No newline at end of file
diff --git a/scripts/jenkins_perf.sh b/scripts/jenkins_perf.sh
new file mode 100755
index 000000000..355b34eec
--- /dev/null
+++ b/scripts/jenkins_perf.sh
@@ -0,0 +1,27 @@
+#!/usr/bin/env bash
+
+# Run performance in jenkins build
+
+. ./env/bin/activate
+
+echo "Run performance test on $(hostname)"
+
+if [ -z %BUILD_NUMBER ]; then
+    echo "Environment variable BUILD_NUMBER is missing."
+    exit 1
+fi
+
+version=$(printf '%.8d' $BUILD_NUMBER)
+echo "Version number: $version"
+
+echo 'Before install'
+which az
+pip list
+
+build_folder=/var/build_share/$BRANCH_NAME/$version
+echo "Install build from $build_folder"
+
+python -m pip install azure-cli --find-links file://$build_folder/artifacts/build -v
+python -m pip freeze
+
+python ./scripts/performance/measure.py
diff --git a/scripts/performance/measure.py b/scripts/performance/measure.py
new file mode 100644
index 000000000..8cf6ad2b5
--- /dev/null
+++ b/scripts/performance/measure.py
@@ -0,0 +1,48 @@
+# --------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+# --------------------------------------------------------------------------------------------
+
+import sys
+from subprocess import check_output, STDOUT
+
+def mean(data):
+    """Return the sample arithmetic mean of data."""
+    n = len(data)
+    if n < 1:
+        raise ValueError('len < 1')
+    return sum(data)/float(n)
+
+
+def sq_deviation(data):
+    """Return sum of square deviations of sequence data."""
+    c = mean(data)
+    return sum((x-c)**2 for x in data)
+
+
+def pstdev(data):
+    """Calculates the population standard deviation."""
+    n = len(data)
+    if n < 2:
+        raise ValueError('len < 2')
+    ss = sq_deviation(data)
+    return (ss/n) ** 0.5
+
+
+real = []
+user = []
+syst = []
+loop = 100
+
+for i in range(loop):
+    lines = check_output(['time -p az'], shell=True, stderr=STDOUT).split('\n')
+    real_time = float(lines[-4].split()[1])
+    real.append(float(lines[-4].split()[1]))
+    user.append(float(lines[-3].split()[1]))
+    syst.append(float(lines[-2].split()[1]))
+    sys.stdout.write('Loop {} => {} \n'.format(i, real_time))
+    sys.stdout.flush()
+
+print('Real: mean => {} \t pstdev => {}'.format(mean(real), pstdev(real)))
+print('User: mean => {} \t pstdev => {}'.format(mean(user), pstdev(user)))
+print('Syst: mean => {} \t pstdev => {}'.format(mean(syst), pstdev(syst)))
diff --git a/src/azure-cli-core/HISTORY.rst b/src/azure-cli-core/HISTORY.rst
index 031f1ad08..edd7d5713 100644
--- a/src/azure-cli-core/HISTORY.rst
+++ b/src/azure-cli-core/HISTORY.rst
@@ -4,6 +4,7 @@ Release History
 ===============
 2.0.3 (unreleased)
 ^^^^^^^^^^^^^^^^^^
+*core: fix a failure when login using a service principal twice (#2800)
 *core: Allow file path of accessTokens.json to be configurable through an env var(#2605)
 *core: Allow configured defaults to apply on optional args(#2703)
 *core: Improved performance
diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py
index 2773418ad..aafb00f25 100644
--- a/src/azure-cli-core/azure/cli/core/_profile.py
+++ b/src/azure-cli-core/azure/cli/core/_profile.py
@@ -499,7 +499,7 @@ class CredsCache(object):
             # pylint: disable=line-too-long
             if (sp_entry.get(_ACCESS_TOKEN, None) != getattr(matched[0], _ACCESS_TOKEN, None) or
                     sp_entry.get(_SERVICE_PRINCIPAL_CERT_FILE, None) != getattr(matched[0], _SERVICE_PRINCIPAL_CERT_FILE, None)):
-                self._service_principal_creds.pop(matched[0])
+                self._service_principal_creds.remove(matched[0])
                 self._service_principal_creds.append(matched[0])
                 state_changed = True
         else:


Test patch:
diff --git a/src/azure-cli-core/tests/test_profile.py b/src/azure-cli-core/tests/test_profile.py
index bbb63cd5f..a78211b84 100644
--- a/src/azure-cli-core/tests/test_profile.py
+++ b/src/azure-cli-core/tests/test_profile.py
@@ -581,6 +581,25 @@ class Test_Profile(unittest.TestCase):  # pylint: disable=too-many-public-method
         self.assertEqual(creds_cache._service_principal_creds, [test_sp, test_sp2])
         mock_open_for_write.assert_called_with(mock.ANY, 'w+')
 
+    @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True)
+    @mock.patch('os.fdopen', autospec=True)
+    @mock.patch('os.open', autospec=True)
+    def test_credscache_add_preexisting_sp_creds(self, _, mock_open_for_write, mock_read_file):
+        test_sp = {
+            "servicePrincipalId": "myapp",
+            "servicePrincipalTenant": "mytenant",
+            "accessToken": "Secret"
+        }
+        mock_open_for_write.return_value = FileHandleStub()
+        mock_read_file.return_value = [test_sp]
+        creds_cache = CredsCache()
+
+        # action
+        creds_cache.save_service_principal_cred(test_sp)
+
+        # assert
+        self.assertEqual(creds_cache._service_principal_creds, [test_sp])
+
     @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True)
     @mock.patch('os.fdopen', autospec=True)
     @mock.patch('os.open', autospec=True)