Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

[feature] Cria funcionalidade context para trabalhar com multiplos contextos #5

Open
rilder-almeida wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[feature] Cria funcionalidade context para trabalhar com multiplos contextos #5

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0fb57c
[WIP] Inicia a implementação de multicontexts
rilder-almeida Sep 15, 2022
62ae279
finalizada implementação da funcionalidade new do menu context
rilder-almeida Sep 20, 2022
90073c0
[feature] Cria funcionalidade context para trabalhar com multiplos co…
rilder-almeida Sep 22, 2022
dd6f108
fix: remove trash files
rilder-almeida Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
236 changes: 222 additions & 14 deletions kcompose.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,34 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.

set -f # Disables Globbing
defaultConfigFile=$HOME/.kcompose/config
configPath=$HOME/.kcompose/
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 29, 2022

Choose a reason for hiding this comment

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

nit: Essa barra no final fez com que as outras configs ficassem "grudadas", ex: ${configPath}default/. Visualmente acharia mais agradável não colocar a barra aqui e definir como ${configPath}/default


defaultConfigPath=${configPath}default/
defaultConfigFile=${defaultConfigPath}config

defaultCredentialsPath=${configPath}default/
defaultCredentialsFile=${defaultCredentialsPath}credentials

if [ ! -f ${configPath}context ]; then
touch ${configPath}context
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 29, 2022

Choose a reason for hiding this comment

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

Acho que não precisa forçar a criação de um arquivo de contexto toda vez. Basta não ler se não estiver criado. Por exemplo, mais abaixo tem:

if [ -f $configFile ]; then
    readConfig
fi

fi

source ${configPath}context
currentContext=${KCOMPOSE_CONTEXT:-"default"}

if [ -z "$KCOMPOSE_CONFIG_FILE" ]; then
if [ ! -d "${configPath}${currentContext}" ]; then
echo -e "WARNING: Context ${currentContext} not found\n\n"
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

Esse warning vai gerar um desconforto no onboarding do usuário, que vai tomar um warning na primeira vez que executar qualquer comando no kcompose. Fora que o primeiro comando a ser executado deveria ser o kcompose setup, e não o kcompose context set.

Tem uma outra questão que eu não entendi muito bem lendo o código. O kcompose possui uma hierarquia de configurações:

default < arquivo de configuração < variável de ambiente

Como os contextos entram aqui? Me parece que o contexto foi implementado de uma forma conflitante ao KCOMPOSE_CONFIG_FILE. Mas até onde eu entendi, tudo o que muda é o valor de defaultConfigFile: Antes, ele era $HOME/.kcompose/config, e com contextos, ele passa ser $HOME/.kcompose/{context}/config, onde o contexto é "default" se nada estiver setado.

Acho que se for feito dessa forma, conseguimos evitar erros desnecessários ao usuário, e funcionar com pouca configuração

echo -e "Please, set a valid context using the command:"
echo -e "kcompose context set <context>\n"
fi
fi

# Config chain
# Defaults
broker="localhost:9092"
credentialsFile=""
kafkaLocation="/usr/share/kcompose/kafka"
credentialsFile=""
configFile=${KCOMPOSE_CONFIG_FILE:-$defaultConfigFile}

# Config file
Expand Down Expand Up @@ -88,6 +109,12 @@ ask() {
eval "$result=\"$answer\""
}

saveContext() {
if [ -f ${configPath}context ]; then
echo "KCOMPOSE_CONTEXT=$1" > ${configPath}context
fi
}

saveConfigs() {
mkdir -p $(dirname $configFile)
cat >$configFile <<EOF
Expand All @@ -97,13 +124,7 @@ KCOMPOSE_KAFKA_LOCATION=$kafkaLocation
EOF
echo
echo "Config files saved on $configFile"
if [ "$configFile" != "$defaultConfigFile" ]; then
echo
echo "Warning! Config files saved on non-default location. Remember to set the environment variable:"
echo
echo "export KCOMPOSE_CONFIG_FILE=$configFile"

fi
echo
}

login() {
Expand All @@ -119,7 +140,11 @@ Authentication type must be one of:
credentialsFile=""
;;
"SASL/SCRAM256")
ask credentialsFile "Credentials File" $HOME/.kcompose/credentials
if [ -z "$KCOMPOSE_CONFIG_FILE" ]; then
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

Acho justo deixar de perguntar ao usuário onde salvar o arquivo de credenciais (é uma etapa meio inútil da configuração), mas acho que isso independe se KCOMPOSE_CONFIG_FILE está setado ou não, podemos sempre salvar dentro da pasta do contexto atual.

credentialsFile=${configPath}$currentContext/credentials
else
credentialsFile=${configPath}credentials
fi
mkdir -p $(dirname $credentialsFile)
ask username "Username" ""
ask password "Password" ""
Expand All @@ -135,7 +160,11 @@ EOF

;;
"SASL/PLAIN")
ask credentialsFile "Credentials File" $HOME/.kcompose/credentials
if [ -z "$KCOMPOSE_CONFIG_FILE" ]; then
credentialsFile=${configPath}$currentContext/credentials
else
credentialsFile=${configPath}credentials
fi
mkdir -p $(dirname $credentialsFile)
ask username "Username" ""
ask password "Password" ""
Expand Down Expand Up @@ -164,11 +193,28 @@ EOF
}

setup() {
ask configFile "Configuration file" $configFile
ask broker "kafka brokers" $broker
if [ -z "$1" ]; then
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

Dica para códigos bash: sempre nomeie os parâmetros das funções, porque se não fica difícil saber o que significa $1.
Por exemplo, na função ask, temos:

local result
local question
local default

result=$1
question=$2
default=$3

ask setupConfig "Configuration name" "default"
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

Pelo o que eu entendi, se a pessoa cair nesse fluxo, é porque ela executou kcompose setup. Se cair no fluxo de baixo, é porque ela executou kcompose context new X, e portanto não precisa de um nome, certo?

Então sugiro trocar "default" aqui por $currentContext, assim se a pessoa já estiver em um contexto, ela pode editar sem precisar lembra o nome do contexto em que está.

else
setupConfig=$1
validate_name $setupConfig
fi
configFile=${configPath}$setupConfig/config

if [ -f $configFile ]; then
ask overwrite "Configuration \"$setupConfig\" already exist, do you want overwrite it? [y/N]" "n"
if [ "$overwrite" != "y" ]; then
exit
fi
fi

currentContext=$setupConfig
saveContext $currentContext

ask broker "Kafka brokers" $broker
ask kafkaLocation "Kafka Location" $kafkaLocation

ask login "login now? (y/n)" "y"
ask login "Login now? (y/n)" "y"

if [ "$login" = "y" ]; then
login
Expand All @@ -177,6 +223,165 @@ setup() {
fi
}

# context is a function that manages the context of the config and credentials files
context() {

if [ ! -z "$KCOMPOSE_CONFIG_FILE" ]; then
echo "Contexts are only available when KCOMPOSE_CONFIG_FILE is not set"
exit 1
fi

helpText="Usage: $programName context [new, set, list, delete, show] [contextName]\n"
helpText+="\tnew\t\tCreate a new context\n"
helpText+="\tset\t\tSet the current context\n"
helpText+="\tlist\t\tList all contexts\n"
helpText+="\tdelete\t\tDelete a context\n"
helpText+="\tshow\t\tShow the current context\n"
helpText+="\tcontextName\tName of the context\n"

# checkNArgs $1
case $1 in
"new")
checkNArgs $2
context_new $2
;;
"set")
checkNArgs $2
context_set $2
;;
"list")
context_list
;;
"delete")
checkNArgs $2
context_delete $2
;;
"show")
context_show
;;
*)
usage
;;
esac
}

# context_new creates a new context
context_new() {
helpText="Usage: $programName context new [contextName]\n"
checkNArgs $1

setup $1
echo "Context \"$1\" was created"
}

# context_set sets the current context
context_set() {
helpText="Usage: $programName context set [contextName]\n"
checkNArgs $1

if [ ! -f ${configPath}$1/config ]; then
echo "Context \"$1\" does not exist"
exit 1
fi
configFile=${configPath}$1/config
currentContext=$1
readConfig
echo "Context set to \"$1\""
saveContext $currentContext
}

# context_list lists all contexts
context_list() {
current=""
echo "Contexts:"
for context in $(ls $configPath); do
if [ -f ${configPath}$context/config ]; then
if [ "$context" = "$currentContext" ]; then
current="*"
else
current=""
fi
echo -e "\t$context$current"
fi
done
}

# context_delete deletes a context
context_delete() {
helpText="Usage: $programName context delete [contextName]\n"
checkNArgs $1

if [ ! -f ${configPath}$1/config ]; then
echo "Context \"$1\" does not exist"
exit 1
fi
rm -rf ${configPath}$1 && echo "Context \"$1\" was deleted"
currentContext="default"
saveContext $currentContext
echo -e "Current context is \"$currentContext\"\n"
echo "If you want to change it, use the command:"
echo "$programName context set [contextName]"
}

# context_show shows the current context
context_show() {
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

nit: Isso aqui parece ter o mesmo efeito do kcompose env, mas para mostrar o conteúdo de um contexto que não o atual. Será que dá pra extrair uma função de lá, e fazer os dois fluxos chamarem a mesma função? Ou pelo menos manter o output similar, para tentar manter uma experiência consistente para o usuário?

if [ ! -d "${configPath}${currentContext}" ]; then
echo "Context \"$currentContext\" does not exist"
echo "Please, set a valid context"
echo "Use the command:"
echo "$programName context set [contextName]"
exit 1
fi
echo -e "Current context: $currentContext\n"
echo -e - "Config file: $configFile\n"
cat $configFile
echo ""
echo -e - "Credentials file: $credentialsFile\n"
cat $credentialsFile
}

# validate_name checks if the filename is valid
validate_name() {
__name=$1
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

o bash tem o conceito de variável "local", talvez dê pra usar isso no lugar de __

val=$(echo "${#__name}")
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

Onde isso está sendo usado?


if [[ $__name == "default" ]]; then
echo "Context name cannot be 'default'"
exit
Copy link
Copy Markdown
Contributor

@andremissaglia andremissaglia Sep 30, 2022

Choose a reason for hiding this comment

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

importante sair com um código diferente de zero. Isso é um padrão unix para indicar que algo deu errado

fi

if [[ $__name == "" ]]; then
echo "Filename: $__name cannot be empty"
exit
fi

if [[ $__name == "." ]] || [[ $__name == ".." ]]; then
# "." and ".." are added automatically and always exist, so you can't have a
# file named . or .. // https://askubuntu.com/a/416508/660555
echo "Filename: $__name is not valid"
exit
fi

if [ $val -gt 255 ]; then
# String's length check
echo "Filename: $__name has more than 255 characters"
exit
fi

if ! [[ $__name =~ ^[0-9a-zA-Z._-]+$ ]]; then
# Checks whether valid characters exist
echo "Filename: $__name has invalid characters"
exit
fi

___name=$(echo $__name | cut -c1-1)
if ! [[ $___name =~ ^[0-9a-zA-Z.]+$ ]]; then
# Checks the first character
echo "Filename: $__name has invalid first character"
exit
fi
}

# commands
helpText="Usage: $programName [topic, acl, produce, consume, group, env, config, setup, login, user]\n"
helpText+="\ttopic\t\tTopic management\n"
Expand Down Expand Up @@ -520,6 +725,9 @@ Examples:
;;
esac
;;
"context")
context $2 $3
;;
*)
usage
;;
Expand Down