Implementa suporte a campos bool com default value by daltonmatos · Pull Request #22 · lukerops/manifest.tf

daltonmatos · 2024-10-15T20:52:41Z

Implementa suporte a default values para camposs boolean.

ref: #16

Tarefas

Adicionar validação para confirmar que o valor que foi escolhido como default é compatível com o tipo do campo. Exemplo: Não podemos fazer default: 42 para um campo type: bool
Confirmar se vamos validar a possibilidade de passarmos default: null para um campo type: bool

bool

root_objects

Os campos dentro do validations do bool/v1 são obrigatório. A implementação do bool/v1 espera que eles existam.

teste

lukerops · 2024-10-22T18:12:11Z

+output "schema" {
+  value = {
+    type        = "object"
+    version     = "v1"


Porque você está adicionando versões diferentes para o object se apenas a lógica do bool será alterada nesse PR? Você fez isso para os outros tipos também.

O que observei foi o seguinte:

A implementação do object/v0 não permite que um campo não exista no var.manifest que está chegando, pois ele faz esse check com o local.missing_properties. Já o object/v1 é mais frouxo nessa validação e permite que alguns campos não existam no manifest, pois sabe que alguins deles terão seus valores preenchidos com um valor padão.
Depois que tivermos todos os tipos de campos podendo ter default values, as duas implementações voltarão a ser iguals e poderemos unificar novamente. Acho inclusive que poderemos remover completamente a lógica do local.missing_properties.

lukerops · 2024-11-25T12:31:20Z

+    condition     = !(
+      local.declared_default_field && try(var.manifest.default, null) == null
+  )


Aqui não deveria ser tem declaração do default e o default for diferente de null ou não tem declaração do default (condition = (local.declared_default_field && try(var.manifest.default, null) != null) || !local.declared_default_field ?

Se colocarmos essa validação (a parte final depois do ||), quando o campo default não for declarado teremos um erro, certo?

Se de fato isso acontecer estaremos tornando a presença do campo default obrigatória, mas declarar ou não um valor default para um campo deveria ser opcional.

A ideia da validação como ela está agora é: "Se um valor default tiver sido declarado, ele não pode ser ``"null".
Faz sentido?

(Posso tentar escrever um teste pra confirmar esse meu pensamento depois)

Outro ponto: Se tirarmos a parte final do "or" nossas expressões são equivalente (eu acho). O que eu geralmente faço, pois acho mais simples de entender, é colocar entre parentes a condição que não podemos ter, e depois faço a negação disso. A negação existe pois o teste falha quando a condition é false e não true.

lukerops · 2024-11-25T12:41:45Z

+    #condition     = local.has_default_value || var.manifest != null
+    condition = !(
+      !local.has_default_value && var.manifest == null
+    )


Faltou remover o comentário.
Aqui não deveria ser tem o default ou não tem default e o manifest é diferente de null (condition = local.has_default_value || (!local.has_default_value && var.manifest != null)`)?

Feito. 4eab358

Sobre a condição, elas são equivalentes, apenas uma está negada e outra não. O que faço aqui é, ter entre parenteses, a condição que representa o erro e depois eu faço a negação de tudo. Faço isso para que fique mais claro a condição que não queremos ter, e a negação final existe pela forma como o teste terraform funciona (quebrando quando a condition é false).

lukerops · 2024-11-25T12:45:08Z

+    subItem = null
+    validations = {
+      has_default_value = can(var.manifest.default) ? true : false
+      default_value     = try(var.manifest.default, null)


O schema deveria carregar o campo já em seu valor final. Com isso em mente, a validação se o campo default possui um valor válido, deveria estar aqui.

Suggested change

default_value = try(var.manifest.default, null)

default_value = try(tobool(var.manifest.default), null)

Feito. 7e3f2a6
ref: 7e3f2a6#diff-568a0a74522ee519a741d9e42c31865650d738a3254efa5098754b71d3082294

lukerops · 2024-11-25T12:46:30Z

+  precondition {
+    condition     = local.has_compatible_default_value
+    error_message = <<-EOT
+      Invalid resource manifest!
+      The default value of the property "${var.field_path}" must be compatible with type bool.
+      Current default value: ${format("%v", var.schema.validations.default_value)}
+      (metadata.name: "${var.metadata_name}"; path: "${var.path}")
+    EOT
+  }


Essa validação deveria estar no processamento do CRD e não no processamento do manifesto final.

Feito. 7e3f2a6

lukerops · 2024-11-25T12:48:03Z

+locals {
+  has_default_value            = var.schema.validations.has_default_value
+  has_compatible_default_value = can(tobool(var.schema.validations.default_value))
+
+  default_value = local.has_compatible_default_value ? tobool(var.schema.validations.default_value) : false
+
+  has_valid_value     = var.manifest != null && can(tobool(var.manifest))
+  final_default_value = local.has_compatible_default_value ? local.default_value : try(tobool(var.manifest), null)
+
+
+}


Se o schema vier com o campo default já validado e transformado em seu valor final, essa lógica inteira se faz desnecessária, ficando bem mais simples de saber o valor final.

Feito. 7e3f2a6

lukerops

Só ajuste da mensagem de erro. Mas de resto, acho que está OK.

lukerops · 2025-08-25T12:34:38Z

+    The default value a bool field can't be null;
+    The filed ${var.field_path} has it's default value set to null.


Suggested change

The default value a bool field can't be null;

The filed ${var.field_path} has it's default value set to null.

The default value of a bool field can't be null;

The field "${var.field_path}" has it's default value set to null.

Feito. d7a7eff

daltonmatos added 13 commits October 14, 2024 13:44

Primeiros testes para o bool do CRD v1alpha2

8043ba6

Duplicando cada teste dos tipos de dados para o v1alhpa2

f50e8b9

testes simpels para confirmar a capacidade de parsear CRDs v1alpha2

b50f506

Confirmando o uso de bool/v1 e root_object/v1 no CRD v1alpha2

33c84fa

Ajustando testes para campos do CRD v1alpha2

c6cfb92

Adiciona testes para bool/v1. Suporte a default value

33769ae

Implementa testes simples para a validação de múltiplas versões do campo

6e9b131

bool

Testes para confirmar que conseguimos validar múltiplas versões de

0264d1c

root_objects

Fix schemaValidation/bool

dd9d817

Os campos dentro do validations do bool/v1 são obrigatório. A implementação do bool/v1 espera que eles existam.

Testes para root_object/v1 com campo bool/v1 (usando default value)

bf2af67

Suporte a bool/v1 para object dentro de root_object

1d15460

Implementa suporte a bool/v1 para reduced_object

c60ef41

Implementa testes e2e para campos boolean com valor default

6288db8

daltonmatos requested a review from lukerops October 15, 2024 20:53

Terraform 1.6.x não permite chamada de função dentro de um arquivo de

60f3e5f

teste

daltonmatos mentioned this pull request Oct 18, 2024

Implementar suporte a default value #16

Open

lukerops reviewed Oct 22, 2024

View reviewed changes

Comment thread CustomResourceDefinition/v1alpha2/bool/main.tf

daltonmatos added 2 commits October 25, 2024 10:02

Implementa validação para um default value incompatível com o tipo bool

8f1f89e

Não permite um valor default de campo bool ser null

b53eac8

daltonmatos marked this pull request as ready for review October 28, 2024 13:24

daltonmatos requested a review from lukerops October 29, 2024 11:38

lukerops requested changes Nov 25, 2024

View reviewed changes

daltonmatos added 7 commits August 16, 2025 11:54

Ajusta mensagem de validação para inglês.

fc8f6df

Remove output de debug.

ca96aae

Removendo comentário que ficou de debugs antigos

4eab358

Fix message for bool default value being null

6fd11de

Removing test that used debug output

438acc4

bool: Move validação do default value para dentro do CRD

7e3f2a6

bool: Simplificando o locals do schemaValidation

e7d158f

daltonmatos requested a review from lukerops August 18, 2025 17:32

lukerops reviewed Aug 25, 2025

View reviewed changes

Fix validation message

d7a7eff

daltonmatos requested a review from lukerops August 25, 2025 14:07

lukerops merged commit 4330f15 into main Aug 25, 2025
4 checks passed

lukerops deleted the feat/implementa-default-value-boolean branch August 25, 2025 18:00

	default_value = try(var.manifest.default, null)
	default_value = try(tobool(var.manifest.default), null)

		The default value a bool field can't be null;
		The filed ${var.field_path} has it's default value set to null.

Conversation

daltonmatos commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tarefas

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daltonmatos Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukerops left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

daltonmatos commented Oct 15, 2024 •

edited

Loading

daltonmatos Aug 16, 2025 •

edited

Loading