Skip to content

Config-file: remove User-Agent from config.json when saving#2755

Merged
thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah:dont_save_useragent
Oct 15, 2020
Merged

Config-file: remove User-Agent from config.json when saving#2755
thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah:dont_save_useragent

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Sep 29, 2020 •
edited
Loading

The config.json allows for setting custom HTTP headers, but given that
User-Agent is not customizable, we should remove it from the config before saving;

Before this change;

$ cat ~/.docker/config.json
{
    "auths": {
        "https://index.docker.io/v1/": {
            "auth": "<base64 auth>"
        }
    },
    "HttpHeaders": {
        "User-Agent": "Docker-Client/19.03.12 (linux)"
    }
}

$ docker logout
{
    "auths": {},
    "HttpHeaders": {
        "User-Agent": "Docker-Client/19.03.12 (linux)"
    }
}

After this change:

$ cat ~/.docker/config.json
{
    "auths": {
        "https://index.docker.io/v1/": {
            "auth": "<base64 auth>"
        }
    },
    "HttpHeaders": {
        "User-Agent": "Docker-Client/19.03.12 (linux)"
    }
}

$ docker logout
Removing login credentials for https://index.docker.io/v1/

$ cat ~/.docker/config.json
{
    "auths": {}
}

@thaJeztah thaJeztah marked this pull request as draft September 29, 2020 14:35
@thaJeztah thaJeztah changed the title config-file: don't save User-Agent to config.json Config-file: remove User-Agent from config.json when saving Sep 29, 2020
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 29, 2020 •
edited
Loading

Relates to #2756, which fixes the cause of the User-Agent ending up in the config.json in the first place

@thaJeztah thaJeztah marked this pull request as ready for review September 29, 2020 15:27
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 29, 2020

@silvin-lubecki PTAL - I'm a bit on the fence if we should actively remove

@silvin-lubecki
Copy link
Copy Markdown
Contributor

silvin-lubecki commented Sep 30, 2020

Looks like a test is failing:

=== Failed
=== FAIL: cli/command/container TestRemoveForce/without_force (0.00s)
    --- FAIL: TestRemoveForce/without_force (0.00s)
        rm_test.go:37: assertion failed: 
            --- removed
            +++ →
            {[]string}[?->1]:
            	-: <non-existent>
            	+: "nosuchcontainer"
            

=== FAIL: cli/command/container TestRemoveForce (0.00s)

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 30, 2020

Hmmm.. interesting; will have to look what's causing that

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 30, 2020 •
edited
Loading

Codecov Report

Merging #2755 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2755   +/-   ##
=======================================
  Coverage   57.15%   57.15%           
=======================================
  Files         297      297           
  Lines       18656    18658    +2     
=======================================
+ Hits        10662    10664    +2     
  Misses       7132     7132           
  Partials      862      862

silvin-lubecki
silvin-lubecki approved these changes Sep 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

I think it's ok and safe to remove those headers which should have never been there 👍

@thaJeztah
Config-file: remove User-Agent from config.json when saving
3f19902 
The config.json allows for setting custom HTTP headers, but given that
User-Agent is not customizable, we should remove it from the config before saving;

Before this change;

    $ cat ~/.docker/config.json
    {
        "auths": {
            "https://index.docker.io/v1/": {
                "auth": "<base64 auth>"
            }
        },
        "HttpHeaders": {
            "User-Agent": "Docker-Client/19.03.12 (linux)"
        }
    }

    $ docker logout
    {
        "auths": {},
        "HttpHeaders": {
            "User-Agent": "Docker-Client/19.03.12 (linux)"
        }
    }

After this change:

    $ cat ~/.docker/config.json
    {
        "auths": {
            "https://index.docker.io/v1/": {
                "auth": "<base64 auth>"
            }
        },
        "HttpHeaders": {
            "User-Agent": "Docker-Client/19.03.12 (linux)"
        }
    }

    $ docker logout
    Removing login credentials for https://index.docker.io/v1/

    $ cat ~/.docker/config.json
    {
        "auths": {}
    }

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the dont_save_useragent branch from 1acbb7b to 3f19902 Compare October 1, 2020 13:04
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Oct 15, 2020

let's merge this one 👍

@thaJeztah thaJeztah merged commit 9b3eef5 into docker:master Oct 15, 2020
@thaJeztah thaJeztah deleted the dont_save_useragent branch October 15, 2020 20:18
@thaJeztah thaJeztah added this to the 20.10.0 milestone Oct 15, 2020
@thaJeztah thaJeztah mentioned this pull request Jun 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@silvin-lubecki silvin-lubecki silvin-lubecki approved these changes

Assignees

No one assigned

Labels

status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah @silvin-lubecki @codecov-commenter